On Tue, Apr 02, 2024 at 09:18:44PM +0800, Xiaoyao Li wrote:
> On 4/2/2024 6:02 PM, Michael S. Tsirkin wrote:
> > On Tue, Apr 02, 2024 at 04:25:16AM -0400, Xiaoyao Li wrote:
> > > Set MADT.FLAGS[bit 0].PCAT_COMPAT based on x86ms->pic.
> > >
> > > Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
> >
> > Please include more info in the commit log:
> > what is the behaviour you observe, why it is wrong,
> > how does the patch fix it, what is guest behaviour
> > before and after.
>
> Sorry, I thought it was straightforward.
>
> A value 1 of PCAT_COMPAT (bit 0) of MADT.Flags indicates that the system
> also has a PC-AT-compatible dual-8259 setup, i.e., the PIC.
>
> When PIC is not enabled for x86 machine, the PCAT_COMPAT bit needs to be
> cleared. Otherwise, the guest thinks there is a present PIC even it is
> booted with pic=off on QEMU.
>
> (I haven't seen real issue from Linux guest. The user of PIC inside guest
> seems only the pit calibration. Whether pit calibration is triggered depends
> on other things. But logically, current code is wrong, we need to fix it
> anyway.
>
> @Isaku, please share more info if you have)
>
That's sufficient, thanks! Pls put this in commit log and resubmit.
> > The commit log and the subject should not repeat
> > what the diff already states.
> >
> > > ---
> > > hw/i386/acpi-common.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> > > index 20f19269da40..0cc2919bb851 100644
> > > --- a/hw/i386/acpi-common.c
> > > +++ b/hw/i386/acpi-common.c
> > > @@ -107,7 +107,9 @@ void acpi_build_madt(GArray *table_data, BIOSLinker
> > > *linker,
> > > acpi_table_begin(&table, table_data);
> > > /* Local APIC Address */
> > > build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4);
> > > - build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /*
> > > Flags */
> > > + /* Flags. bit 0: PCAT_COMPAT */
> > > + build_append_int_noprefix(table_data,
> > > + x86ms->pic != ON_OFF_AUTO_OFF ? 1 : 0 , 4);
> > > for (i = 0; i < apic_ids->len; i++) {
> > > pc_madt_cpu_entry(i, apic_ids, table_data, false);
> > > --
> > > 2.34.1
> >
