Re: [PATCH 4/5] hw/char/serial-isa.c: declare IRQ as shared in ACPI IRQ descriptor

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 4 Mar 2026 14:36:14 +0000
Mark Cave-Ayland <mark.caveayland@nutanix.com> wrote:

> On 04/03/2026 11:22, Igor Mammedov wrote:
> 
> > On Fri, 27 Feb 2026 13:44:58 +0000
> > Mark Cave-Ayland <mark.caveayland@nutanix.com> wrote:
> >   
> >>  From Windows 8.1 onwards ISA serial IRQs cannot be shared when ACPI Revision
> >> 5.0 is used in the FACP table. The reason for this is that if a 2-byte IRQ
> >> Descriptor is used then the interrupt is considered to be high true, edge
> >> sensitive, non-shareable. Since legacy serial ports COM1/3 and COM2/4 share
> >> an IRQ then if more than 2 serial ports are added, Windows indicates a
> >> conflict in Device Manager and these combinations cannot be used together.
> >>
> >> Add a new 3-byte IRQ Descriptor to the _CRS resource indicating that the
> >> ISA serial IRQ is low true, edge sensitive and shareable, along with a
> >> corresponding _PRS resource so that the legacy serial ports also appear
> >> at a fixed address. This enables all 4 legacy serial ports to be used in
> >> Windows without conflict.  
> > 
> > What happens if we just replace aml_irq_no_flags() with aml_irq()
> > (without compat knob and _PRS)
> > 
> > wrt _PRS could you elaborate some more why it's needed and what happens
> > if it doesn't exists?  
> 
> Good question. I based the implementation on the technote from Microchip 
> at https://ww1.microchip.com/downloads/en/DeviceDoc/00001879A.pdf and 
> found that it worked fine here on Windows 11.
> 
> My concern from deviating from the document would be that any changes 
> would work fine on Windows 11, but then fail on older versions of Windows.
> 
> I can try and locate a copy of Windows 8.1 internally if you still think 
> that is worth pursuing?

with _CRS present, I don't think we need _PRS especially in absence of _SRS
and means to actually changes used IRQs/IO.

What I'd like to avoid is adding not needed code and compat logic
if it's possible.
The later unfortunately achievable only by tedious testing of
the change with older Windows versions (the older it is,
the more loose spec interperetation).

> 
> >> Finally add a new x-acpi-shared-irq property to disable the ACPI IRQ descriptor
> >> changes for older PC machine types, and add it to the pc_compat_10_2[] array.
> >>
> >> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> >> ---
> >>   hw/char/serial-isa.c | 23 ++++++++++++++++++++++-
> >>   hw/i386/pc.c         |  4 +++-
> >>   2 files changed, 25 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> >> index a4be0492c5..1662da86bd 100644
> >> --- a/hw/char/serial-isa.c
> >> +++ b/hw/char/serial-isa.c
> >> @@ -28,6 +28,7 @@
> >>   #include "qemu/module.h"
> >>   #include "system/system.h"
> >>   #include "hw/acpi/acpi_aml_interface.h"
> >> +#include "hw/acpi/aml-build.h"
> >>   #include "hw/char/serial.h"
> >>   #include "hw/char/serial-isa.h"
> >>   #include "hw/isa/isa.h"
> >> @@ -43,6 +44,7 @@ struct ISASerialState {
> >>       uint32_t index;
> >>       uint32_t iobase;
> >>       uint32_t isairq;
> >> +    bool acpi_shared_irq;
> >>       SerialState state;
> >>   };
> >>   
> >> @@ -92,7 +94,12 @@ static void serial_isa_build_aml(AcpiDevAmlIf *adev, Aml *scope)
> >>   
> >>       crs = aml_resource_template();
> >>       aml_append(crs, aml_io(AML_DECODE16, isa->iobase, isa->iobase, 0x00, 0x08));
> >> -    aml_append(crs, aml_irq_no_flags(isa->isairq));
> >> +    if (isa->acpi_shared_irq) {
> >> +        aml_append(crs, aml_irq(isa->isairq, AML_EDGE, AML_ACTIVE_LOW,
> >> +                                AML_SHARED));
> >> +    } else {
> >> +        aml_append(crs, aml_irq_no_flags(isa->isairq));
> >> +    }
> >>   
> >>       dev = aml_device("COM%d", isa->index + 1);
> >>       aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0501")));
> >> @@ -100,6 +107,18 @@ static void serial_isa_build_aml(AcpiDevAmlIf *adev, Aml *scope)
> >>       aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> >>       aml_append(dev, aml_name_decl("_CRS", crs));
> >>   
> >> +    if (isa->acpi_shared_irq) {
> >> +        Aml *prs = aml_resource_template();
> >> +
> >> +        aml_append(prs, aml_start_dependent_function(0, 0));
> >> +        aml_append(prs, aml_io(AML_DECODE16, isa->iobase, isa->iobase, 0x00,
> >> +                               0x08));
> >> +        aml_append(prs, aml_irq(isa->isairq, AML_EDGE, AML_ACTIVE_LOW,
> >> +                                AML_SHARED));
> >> +        aml_append(prs, aml_end_dependent_function());
> >> +        aml_append(dev, aml_name_decl("_PRS", prs));
> >> +    }
> >> +
> >>       aml_append(scope, dev);
> >>   }
> >>   
> >> @@ -117,6 +136,8 @@ static const Property serial_isa_properties[] = {
> >>       DEFINE_PROP_UINT32("index",  ISASerialState, index,   -1),
> >>       DEFINE_PROP_UINT32("iobase",  ISASerialState, iobase,  -1),
> >>       DEFINE_PROP_UINT32("irq",    ISASerialState, isairq,  -1),
> >> +    DEFINE_PROP_BOOL("x-acpi-shared-irq", ISASerialState, acpi_shared_irq,
> >> +                     true),
> >>   };
> >>   
> >>   static void serial_isa_class_initfn(ObjectClass *klass, const void *data)
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index 0dd3fd01d9..c0335b05ba 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -82,7 +82,9 @@
> >>       { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
> >>       { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
> >>   
> >> -GlobalProperty pc_compat_10_2[] = {};
> >> +GlobalProperty pc_compat_10_2[] = {
> >> +    { "isa-serial", "x-acpi-shared-irq", "false" },
> >> +};
> >>   const size_t pc_compat_10_2_len = G_N_ELEMENTS(pc_compat_10_2);
> >>   
> >>   GlobalProperty pc_compat_10_1[] = {  
> 
> 
> ATB,
> 
> Mark.
>