From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71 Date: Fri, 25 Sep 2015 10:55:13 +0100 Message-ID: <56051A01.1060306@citrix.com> References: <56016F7B02000078000A45DA@prv-mh.provo.novell.com> <5602F0BE.6070801@citrix.com> <5603CC6D02000078000A521D@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZfPim-0002n9-SQ for xen-devel@lists.xenproject.org; Fri, 25 Sep 2015 09:55:16 +0000 In-Reply-To: <5603CC6D02000078000A521D@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org On 24/09/15 09:11, Jan Beulich wrote: >>>> On 23.09.15 at 20:34, wrote: >> On 22/09/15 14:10, Jan Beulich wrote: >>> + for ( offs = 2; offs < 8; offs <<= 1 ) >>> + { >>> + bool_t read = 1; >>> + >>> + for ( i = RTC_REG_D + 1; i < 0x80; ++i ) >>> + { >>> + uint8_t normal, alt; >>> + unsigned long flags; >>> + >>> + if ( i == acpi_gbl_FADT.century ) >>> + continue; >>> + >>> + spin_lock_irqsave(&rtc_lock, flags); >>> + >>> + normal = CMOS_READ(i); >>> + if ( inb(RTC_PORT(offs)) != i ) >>> + read = 0; >>> + >>> + alt = inb(RTC_PORT(offs + 1)); >>> + >>> + spin_unlock_irqrestore(&rtc_lock, flags); >>> + >>> + if ( normal != alt ) >>> + break; >> Even with a manual to hand, this logic is quite hard to understand. >> Furthermore, I still cant spot how your r/w vs w/o logic is supposed to >> work. It doesn't check the writability of the alias, but of the aliases >> index. > But that's the only interesting thing. A w/o alias would be rather > strange. It's the index register that traditionally hasn't been > readable, but has got means added in some chipsets to be read > back. For the moment we don't make use of this information > anyway, i.e. it's more a thing usable for validation that what the > logic determines matches with how the chipset is documented to > behave (if someone wanted to check that). > >> However, it is not robust to the system servicing an SMI and altering >> the CMOS ram in the middle of this loop. Such a modification would >> cause the loop to believe that this specific 'offs' is not an alias even >> when it actually is. >> >> One option would be to reread the non-aliased port again, but that would >> add yet more io reads. > SMI is always a problem. Even if we re-read the register, we still > couldn't be sure we haven't got hit by another SMI. Considering > the index/data port access model I would anyway consider it quite > bad for firmware to modify the CMOS in an SMI. When one core is executing the SMM handler, all other cores are held paused. (There tends to be some rather complicated cross-socket logic to make this happen properly). It is safe for the SMM handler to use CMOS if it returns the index register back to how it found it. Furthermore, I am willing to bet that there real SMM handlers out there which do do this. ~Andrew