Re: [Xen-devel] [PATCH] acpi: set correct address of the control/event blocks in the FADT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.08.17 at 11:31, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, Aug 29, 2017 at 10:25:23AM +0100, Wei Liu wrote:
>> On Tue, Aug 29, 2017 at 10:17:24AM +0100, Roger Pau Monne wrote:
>> > On Tue, Aug 29, 2017 at 03:08:57AM -0600, Jan Beulich wrote:
>> > > >>> On 29.08.17 at 10:50, <roger.pau@xxxxxxxxxx> wrote:
>> > > > Commit 149c6b unmasked an issue long present in Xen: the control/event
>> > > > block addresses provided in the ACPI FADT table where hardcoded to the
>> > > > V1 version. This was papered over because hvmloader would also always
>> > > > set HVM_PARAM_ACPI_IOPORTS_LOCATION to 1 regardless of the BIOS
>> > > > version.
>> > > > 
>> > > > Fix this by passing the address of the control/event blocks to
>> > > > acpi_build_tables, so the values can be properly set in the FADT
>> > > > table provided to the guest.
>> > > > 
>> > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> > > 
>> > > The patch here looks good, i.e. could have my R-b, just that it is
>> > > entirely unclear to me how things did work before the quoted
>> > > commit: Ports used by Xen and qemu-trad must have been out of
>> > > sync, or am I overlooking something?
>> > 
>> > Yes, the GPE port used by qemu-trad was out of sync with the one
>> > reported in the FADT.
>> > 
>> > AFAICT the only thing that didn't work with qemu-trad was ACPI vCPU
>> > hotplug, but we don't test that in osstest (not even sure if we test
>> > xenstore vCPU hotplug).
>> > 
>> > PM1a and TMR worked fine because the V1 address was hardcoded in the
>> > FADT, and HVM_PARAM_ACPI_IOPORTS_LOCATION was unconditionally set to 1
>> > by hvmloader.
>> 
>> Do you maybe want to put some of the above into the commit message?
>> 
>> You can provide me a new one here in a reply, no need to resend. I want
>> to cimmit this asap.
> 
> OK, I think the following is clearer:
> 
> Commit 149c6b unmasked an issue long present in Xen: the control/event
> block addresses provided in the ACPI FADT table where hardcoded to the
> V1 version. This was papered over because hvmloader would also always
> set HVM_PARAM_ACPI_IOPORTS_LOCATION to 1 regardless of the BIOS
> version.
> 
> The most notable issue caused by the above bug was that the QEMU
> traditional GPE0 block was out of sync: the address provided in the
> FADT didn't match the address QEMU was using.
> 
> Note that PM1a and TMR worked fine because the V1 address was
> hardcoded in the FADT and HVM_PARAM_ACPI_IOPORTS_LOCATION was
> unconditionally set to 1 by hvmloader.
> 
> Fix this by passing the address of the control/event blocks to
> acpi_build_tables, so the values can be properly set in the FADT table
> provided to the guest.

LGTM

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel