All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Bandan Das <bsd@redhat.com>
Cc: kvm@vger.kernel.org
Subject: Re: APIC_ID in apic_reg_write()
Date: Thu, 30 Apr 2015 07:40:20 +0200	[thread overview]
Message-ID: <5541C044.1080408@siemens.com> (raw)
In-Reply-To: <jpgy4la379d.fsf@redhat.com>

Am 2015-04-30 um 00:21 schrieb Bandan Das:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> ...
>>>
>>> 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.
>>
>>>
>>> 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 :)
>>
>> 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

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?

> printk_once warning 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.

> 
> 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

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

  reply	other threads:[~2015-04-30  5:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-29 16:47 APIC_ID in apic_reg_write() Bandan Das
2015-04-29 18:34 ` Jan Kiszka
2015-04-29 18:54   ` Bandan Das
2015-04-29 19:07     ` Jan Kiszka
2015-04-29 22:21       ` Bandan Das
2015-04-30  5:40         ` Jan Kiszka [this message]
2015-04-30 16:50           ` Bandan Das

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5541C044.1080408@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=bsd@redhat.com \
    --cc=kvm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.