APIC_ID in apic_reg_write()
Why do we allow writes to APIC_ID ? On all _newer_ processors, it's read only. The spec doesn't explicitly mention it though, or atleast I couldn't find it. Does userspace have a reason to modify it ?
Bandan
Am 2015-04-29 um 18:47 schrieb Bandan Das:
Quoted 4 lines
Why do we allow writes to
APIC_ID? On all _newer_ processors, it's read only. The spec doesn't explicitly mention it though, or atleast I couldn't find it. Does userspace have a reason to modify it ?
The APIC ID is read-only for x2APIC. It remains R/W for xAPIC.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Quoted 7 lines · Jan Kiszka
Am 2015-04-29 um 18:47 schrieb Bandan Das:
Quoted 4 lines
Why do we allow writes to
APIC_ID? On all _newer_ processors, it's read only. The spec doesn't explicitly mention it though, or atleast I couldn't find it. Does userspace have a reason to modify it ?The APIC ID is read-only for x2APIC. It remains R/W for xAPIC.
Are you sure ? In 10.4 of the SDM, there is Note that says
"In processors based on Intel microarchitecture code name Nehalem the Local APIC ID Register is no longer Read/Write; it is Read Only."
And I can verify on a SandyBridge and Haswell system that it's RO there too.
In fact, that was one of the reasons I had submitted a patch to remove verify_local_APIC() from x86/kernel/apic.c (4399c03c678) If I am wrong we need to revert atleast the associated commit message :)
Quoted 1 line
Jan
Am 2015-04-29 um 20:54 schrieb Bandan Das:
Quoted 8 lines
Quoted 6 lines · Jan Kiszka
Am 2015-04-29 um 18:47 schrieb Bandan Das:
Quoted 4 lines
Why do we allow writes to
APIC_ID? On all _newer_ processors, it's read only. The spec doesn't explicitly mention it though, or atleast I couldn't find it. Does userspace have a reason to modify it ?
Quoted 5 lines
Quoted 1 line
The APIC ID is read-only for x2APIC. It remains R/W for xAPIC.
Are you sure ? In 10.4 of the SDM, there is Note that says
"In processors based on Intel microarchitecture code name Nehalem the Local APIC ID Register is no longer Read/Write; it is Read Only."
Indeed - confusing, specifically as it is placed there without a direct link to the corresponding row in the table. And it doesn't say "since" or so.
Quoted 2 lines
And I can verify on a SandyBridge and Haswell system that it's RO there too.
So the APIC just ignores the writes, it doesn't through #GP at least.
Quoted 4 lines
In fact, that was one of the reasons I had submitted a patch to remove
verify_local_APIC()fromx86/kernel/apic.c(4399c03c678) If I am wrong we need to revert atleast the associated commit message :)
Well, we can't remove APIC ID modification support from KVM, though, because older CPU types we may want to emulate correctly had that feature. But we may have to make it configurable to ensure accurate behaviour.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Jan Kiszka <[email protected]> writes:
...
Quoted 6 lines
Quoted 2 lines
And I can verify on a SandyBridge and Haswell system that it's RO there too.
So the APIC just ignores the writes, it doesn't through #GP at least.
Quoted 1 line
Quoted 8 lines
Quoted 3 lines
In fact, that was one of the reasons I had submitted a patch to remove
verify_local_APIC()fromx86/kernel/apic.c(4399c03c678) If I am wrong we need to revert atleast the associated commit message :)Well, we can't remove APIC ID modification support from KVM, though, because older CPU types we may want to emulate correctly had that feature. But we may have to make it configurable to ensure accurate behaviour.
IMO we should just mark it as read-only. 10.4.6 2nd para says -
"In MP systems, the local APIC ID is also used as a processor ID by the BIOS and the operating system. Some processors permit software to modify the APIC ID. However, the ability of software to modify the APIC ID is processor model specific. Because of this, operating system software should avoid writing to the local APIC ID register."
Not that marking it read-only has any huge benefit, but a r/w ID reg could be a source of bugs with misbehaving guests. Or at the least, a printk_once warning message when userspace attempts to modify it. Moreover, we do make an exception with enabling x2apic for guests.
Setting r/w permissions on a per-model is little overkill, don't you think ?
Quoted 1 line
Jan
Am 2015-04-30 um 00:21 schrieb Bandan Das:
Quoted 19 lines
Jan Kiszka <[email protected]> writes:
...Quoted 6 lines
Quoted 2 lines
And I can verify on a SandyBridge and Haswell system that it's RO there too.
So the APIC just ignores the writes, it doesn't through #GP at least.
Quoted 1 line
Quoted 8 lines
Quoted 3 lines
In fact, that was one of the reasons I had submitted a patch to remove
verify_local_APIC()fromx86/kernel/apic.c(4399c03c678) If I am wrong we need to revert atleast the associated commit message :)Well, we can't remove APIC ID modification support from KVM, though, because older CPU types we may want to emulate correctly had that feature. But we may have to make it configurable to ensure accurate behaviour.
IMO we should just mark it as read-only. 10.4.6 2nd para says -
Quoted 8 lines
"In MP systems, the local APIC ID is also used as a processor ID by the BIOS and the operating system. Some processors permit software to modify the APIC ID. However, the ability of software to modify the APIC ID is processor model specific. Because of this, operating system software should avoid writing to the local APIC ID register."
Not that marking it read-only has any huge benefit, but a r/w ID reg could be a source of bugs with misbehaving guests. Or at the least, a
The current code has been there for quite a while, accepting writes even for CPU models that don't do this on real hw, and nothing apparently broke - or do you know stories?
Quoted 2 lines
printk_oncewarning message when userspace attempts to modify it. Moreover, we do make an exception with enabling x2apic for guests.
The situation is different with x2apic because we even have to raise #GP in case the guest attempts a write. That's mandated by the spec.
Quoted 2 lines
Setting r/w permissions on a per-model is little overkill, don't you think ?
If we want accurate behaviour, we should do this. If not, we probably better leave the code alone to avoid surprises for preexisting host/guest setups. Modern OSes do not care anyway, but special ones may get unhappy.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Jan Kiszka <[email protected]> writes:
...
Quoted 8 lines
Quoted 3 lines
Setting r/w permissions on a per-model is little overkill, don't you think ?
If we want accurate behaviour, we should do this. If not, we probably better leave the code alone to avoid surprises for preexisting host/guest setups. Modern OSes do not care anyway, but special ones may get unhappy.
Agreed, better to leave it alone.
Quoted 1 line
Jan