On 20.04.2023 16:31, Roger Pau Monné wrote: > On Thu, Apr 20, 2023 at 10:31:08AM +0200, Jan Beulich wrote: >> On 19.04.2023 17:55, Roger Pau Monné wrote: >>> On Wed, Apr 19, 2023 at 03:58:10PM +0200, Jan Beulich wrote: >>>> @@ -1342,6 +1349,17 @@ unsigned int rtc_guest_read(unsigned int >>>> * underlying hardware would permit doing so. >>>> */ >>>> data = currd->arch.cmos_idx & (0xff >> (port == RTC_PORT(0))); >>>> + >>>> + /* >>>> + * When there's (supposedly) no RTC/CMOS, we don't intercept the >>>> other >>>> + * ports. While reading the index register isn't normally >>>> possible, >>>> + * play safe and return back whatever can be read (just in case a >>>> value >>>> + * written through an alias would be attempted to be read back >>>> here). >>>> + */ >>>> + if ( port == RTC_PORT(0) && >>>> + (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) && >>>> + ioports_access_permitted(currd, port, port) ) >>>> + data = inb(port) & 0x7f; >>> >>> Do we really need to mask the high bit here? We don't allow setting >>> that bit in the first place. >> >> I think it's more consistent to mask it off, specifically with the code >> visible in context right above the insertion. The doc isn't really clear >> about readability of that bit: On one hand in says R/W for port 0x70 in >> the NMI_EN section, yet otoh in the RTC section it says "Note that port >> 70h is not directly readable. The only way to read this register is >> through Alt Access mode." (I think the NMI_EN section is more trustworthy, >> but still.) Plus if we were to ever make use of the NMI disable, we >> wouldn't want Dom0 see the bit set. > > I guess so, at the end Xen itself doesn't use the bit so far. Maybe > at some point we would want to expose the value of the bit to dom0 if > Xen starts using it (most than anything for informative purposes if > NMIs are disabled). > > Feel free to fold the diff to the existing patch and keep the RB.
Thanks. > I guess you will also add something to the commit message about the > special handling of the NMI enable bit even when the RTC/CMOS is not > present? Of course, albeit not more than a sentence, as the code comments provide the details. Jan
