Re: [Qemu-arm] [PATCH] hw/arm/acpi: enable SHPC native hot plug

[Heyi Guo]

On 2019/3/1 18:44, Igor Mammedov wrote:
> On Fri, 1 Mar 2019 10:28:30 +0800
> Heyi Guo <address@hidden> wrote:
>
> > After the introduction of generic PCIe root port and PCIe-PCI bridge,
> > we will also have SHPC controller on ARM, so just enalbe SHPC native
> > hot plug.
> Just out of curiosity,
> An understand the need for SHPC on plain PCI but in case of PCIe
> why native PCIe hotplug isn't sufficient?
For future extension and completeness:
1. PCI is sometimes more flexible than PCIe, for it can have up to 32 devices 
in virtual world. The shortage for PCIe hot plug is that the number of PCIe 
root ports is fixed.
2. We have SHPC when booting through device tree mode, but don't have it 
through ACPI mode.

If I missed something, please let me know.

Thanks,
Heyi

> > Cc: Shannon Zhao <address@hidden>
> > Cc: Peter Maydell <address@hidden>
> > Cc: "Michael S. Tsirkin" <address@hidden>
> > Cc: Igor Mammedov <address@hidden>
> > Signed-off-by: Heyi Guo <address@hidden>
> > Signed-off-by: Heyi Guo <address@hidden>
> > ---
> >   hw/arm/virt-acpi-build.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 04b62c7..7849ec5 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
> > MemMapEntry *memmap,
> >           aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> >       aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> >       aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> > -    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
> > +
> > +    /*
> > +     * Allow OS control for all 5 features:
> > +     * PCIeHotplug SHPCHotplug PME AER PCIeCapability.
> > +     */
> > +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1F), NULL),
> >                                   aml_name("CTRL")));
> >   
> >       ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
>
> .