Re: [PATCH 1/2] arm/virt/acpi: remove meaningless sub device "PR0" from PCI0

[Guoheyi]

在 2020/1/13 20:37, Igor Mammedov 写道:
> On Thu, 19 Dec 2019 14:47:58 +0800
> Heyi Guo <address@hidden> wrote:
>
> > The sub device "PR0" under PCI0 in ACPI/DSDT does not make any sense,
> > so simply remote it.
> Could you make commit message more concrete so it would say
> why it doesn't make any sense.

The reason I'd like to remove it is that I never see a reason that it 
should be there :)

1. I searched through ACPI spec and PCI firmware spec, but didn't find 
any reason for its existance (might miss something).

2. We don't have such "PR0" device on PCI host bridge object on physical 
arm64 machines and these platforms work well.

3. The VM also works well after removing this device.

4. This device looks strange to only have an identity (_ADR) but not any 
other attribute; also this device was introduced in the initial commit 
without any special comment.

Thanks,

Heyi


> It seems to be there to describe root port,
> I'd rather have PCI folk ack if it's ok to remove it.

> > Signed-off-by: Heyi Guo <address@hidden>
> >
> > ---
> > Cc: Peter Maydell <address@hidden>
> > Cc: "Michael S. Tsirkin" <address@hidden>
> > Cc: Igor Mammedov <address@hidden>
> > Cc: Shannon Zhao <address@hidden>
> > Cc: address@hidden
> > Cc: address@hidden
> > ---
> >   hw/arm/virt-acpi-build.c          |   4 ----
> >   tests/data/acpi/virt/DSDT         | Bin 18462 -> 18449 bytes
> >   tests/data/acpi/virt/DSDT.memhp   | Bin 19799 -> 19786 bytes
> >   tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18449 bytes
> >   4 files changed, 4 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index bd5f771e9b..9f4c7d1889 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -317,10 +317,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
> > MemMapEntry *memmap,
> >       aml_append(method, aml_return(buf));
> >       aml_append(dev, method);
> >   
> > -    Aml *dev_rp0 = aml_device("%s", "RP0");
> > -    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> > -    aml_append(dev, dev_rp0);
> > -
> >       Aml *dev_res0 = aml_device("%s", "RES0");
> >       aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
> >       crs = aml_resource_template();
> > diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT
> > index 
> > d0f3afeb134fdf1c11f64cd06dbcdd30be603b80..b5895cb22446860a0b9be3d32ec856feb388be4c
> >  100644
> > GIT binary patch
> > delta 39
> > vcmbO?fpOvlMlP3Nmk>b@1_q`B6S<_Bdg?Z+cXBfI+}XT|v(|R9jr$`2@RSW)
> >
> > delta 50
> > zcmbO@fpOjhMlP3Nmk>D*1_q{tiCof5o%I{lJ2{y;?{412S!>J19TZ>?&k^tF5;R%I
> > G{V4!>hYx%J
> >
> > diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
> > index 
> > 41ccc6431b917252bcbaac86c33b340c796be5ce..69ad844f65d047973a3e55198beecd45a35b8fce
> >  100644
> > GIT binary patch
> > delta 40
> > wcmcaUi}BPfMlP3Nmk=*s1_q}3iCof5t(P{ccXBfI+}XT|v(|RAjk`1(02g)*ivR!s
> >
> > delta 51
> > zcmX>#i}Cs_MlP3NmymE@1_mbiiCof5O_w*ScXBdy-rc;3v(}c2J1D>)o+IATC1|sb
> > HyBr$;t7;Fc
> >
> > diff --git a/tests/data/acpi/virt/DSDT.numamem 
> > b/tests/data/acpi/virt/DSDT.numamem
> > index 
> > d0f3afeb134fdf1c11f64cd06dbcdd30be603b80..b5895cb22446860a0b9be3d32ec856feb388be4c
> >  100644
> > GIT binary patch
> > delta 39
> > vcmbO?fpOvlMlP3Nmk>b@1_q`B6S<_Bdg?Z+cXBfI+}XT|v(|R9jr$`2@RSW)
> >
> > delta 50
> > zcmbO@fpOjhMlP3Nmk>D*1_q{tiCof5o%I{lJ2{y;?{412S!>J19TZ>?&k^tF5;R%I
> > G{V4!>hYx%J
>
> .