arm64: PCI: Add quirk for platforms running Windows

  Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
host bridge windows") introduced a check to remove host bridge register
resources for all arm64 platforms, with the assumption that the PNP0A03
_CRS resources would always be host bridge registers and never as windows
on arm64.

The assumption stands true until Qualcomm Snapdragon Windows laptops
emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
resources instead.  For example, the Microsoft Surface Pro X has host
bridges defined as

    Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
    Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID

    Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
    {
        Name (RBUF, ResourceTemplate ()
        {
            Memory32Fixed (ReadWrite,
                0x60200000,         // Address Base
                0x01DF0000,         // Address Length
                )
            WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
                0x0000,             // Granularity
                0x0000,             // Range Minimum
                0x0001,             // Range Maximum
                0x0000,             // Translation Offset
                0x0002,             // Length
                ,, )
        })
        Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
    }

The Memory32Fixed holds a host bridge window, but it's not properly
defined as a "producer" resource.  Consequently the resource gets
removed by kernel, and the BAR allocation fails later on:

    [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
    [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
    [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
    [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]

This eventually prevents the PCIe NVME drive from being accessible.

Add a quirk for these platforms to avoid the resource being removed.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
We are running into the issue on more devices than just Surface Pro X
now, so trying to sort it out with a quirk as suggested by Lorenzo [1].

[1] https://lore.kernel.org/all/20210527093200.GA16444@lpieralisi/

 arch/arm64/kernel/pci.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
  

Hi,

On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 2276689b5411..896dbd028b67 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -109,16 +109,42 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  	return 0;
>  }
>  
> +#define QCOM_DSDT_QUIRK "Host bridge windows in PNP0A03 _CRS"
> +
> +static struct acpi_platform_list qcom_platlist[] = {
        ^^^
       const

> +	/* Thinkpad X13s */
> +	{ "LENOVO", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> +	/* Microsoft Surface Pro 9 (5G) and Windows Dev Kit 2023 */
> +	{ "QCOMM ", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> +	/* Microsoft Surface Pro X */
> +	{ "QCOMM ", "SDM8180 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> +	{ }
> +};
> +
>  static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
>  {
>  	struct resource_entry *entry, *tmp;
>  	int status;
> +	int idx;
>  
>  	status = acpi_pci_probe_root_resources(ci);
> +
> +	/*
> +	 * Most arm64 platforms that do not run Windows describe host bridge
> +	 * registers in PNP0A03 _CRS resources, but some like Qualcomm
> +	 * Snapdragon Windows laptops describe host bridge windows in there.
> +	 * We do not want to destroy the resources for these platforms.
> +	 */
> +	idx = acpi_match_platform_list(qcom_platlist);
> +	if (idx >= 0)
> +		goto done;
> +
>  	resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
>  		if (!(entry->res->flags & IORESOURCE_WINDOW))
>  			resource_list_destroy_entry(entry);
>  	}
> +
> +done:
>  	return status;
>  }
>  
> -- 
> 2.17.1
>
  

On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
> host bridge windows") introduced a check to remove host bridge register
> resources for all arm64 platforms, with the assumption that the PNP0A03
> _CRS resources would always be host bridge registers and never as windows
> on arm64.
> 
> The assumption stands true until Qualcomm Snapdragon Windows laptops
> emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
> resources instead.  For example, the Microsoft Surface Pro X has host
> bridges defined as
> 
>     Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
>     Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
> 
>     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>     {
>         Name (RBUF, ResourceTemplate ()
>         {
>             Memory32Fixed (ReadWrite,
>                 0x60200000,         // Address Base
>                 0x01DF0000,         // Address Length
>                 )
>             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
>                 0x0000,             // Granularity
>                 0x0000,             // Range Minimum
>                 0x0001,             // Range Maximum
>                 0x0000,             // Translation Offset
>                 0x0002,             // Length
>                 ,, )
>         })
>         Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
>     }
> 
> The Memory32Fixed holds a host bridge window, but it's not properly
> defined as a "producer" resource.  Consequently the resource gets
> removed by kernel, and the BAR allocation fails later on:
> 
>     [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
>     [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
>     [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
>     [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> 
> This eventually prevents the PCIe NVME drive from being accessible.
> 
> Add a quirk for these platforms to avoid the resource being removed.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> We are running into the issue on more devices than just Surface Pro X
> now, so trying to sort it out with a quirk as suggested by Lorenzo [1].

One thing I don't like about this application of quirks is that the
list of affected platforms is likely to grow, which is an ongoing
burden for users and developers.

Can we have a conversation with Qualcomm about how they *intend* this
to work?  Linux is probably doing something wrong (interpreting
something differently than Windows does), and if we could fix that, we
have a better chance of future platforms working without quirks.

> [1] https://lore.kernel.org/all/20210527093200.GA16444@lpieralisi/
> 
>  arch/arm64/kernel/pci.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 2276689b5411..896dbd028b67 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -109,16 +109,42 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  	return 0;
>  }
>  
> +#define QCOM_DSDT_QUIRK "Host bridge windows in PNP0A03 _CRS"
> +
> +static struct acpi_platform_list qcom_platlist[] = {
> +	/* Thinkpad X13s */
> +	{ "LENOVO", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> +	/* Microsoft Surface Pro 9 (5G) and Windows Dev Kit 2023 */
> +	{ "QCOMM ", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> +	/* Microsoft Surface Pro X */
> +	{ "QCOMM ", "SDM8180 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> +	{ }
> +};
> +
>  static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
>  {
>  	struct resource_entry *entry, *tmp;
>  	int status;
> +	int idx;
>  
>  	status = acpi_pci_probe_root_resources(ci);
> +
> +	/*
> +	 * Most arm64 platforms that do not run Windows describe host bridge
> +	 * registers in PNP0A03 _CRS resources, but some like Qualcomm
> +	 * Snapdragon Windows laptops describe host bridge windows in there.
> +	 * We do not want to destroy the resources for these platforms.
> +	 */
> +	idx = acpi_match_platform_list(qcom_platlist);
> +	if (idx >= 0)
> +		goto done;
> +
>  	resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
>  		if (!(entry->res->flags & IORESOURCE_WINDOW))
>  			resource_list_destroy_entry(entry);
>  	}
> +
> +done:
>  	return status;
>  }
>  
> -- 
> 2.17.1
>
  

+ linux-arm-msm and MSM maintainer Bjorn

On Wed, Mar 08, 2023 at 12:53:10PM -0600, Bjorn Helgaas wrote:
> On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> > Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
> > host bridge windows") introduced a check to remove host bridge register
> > resources for all arm64 platforms, with the assumption that the PNP0A03
> > _CRS resources would always be host bridge registers and never as windows
> > on arm64.
> > 
> > The assumption stands true until Qualcomm Snapdragon Windows laptops
> > emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
> > resources instead.  For example, the Microsoft Surface Pro X has host
> > bridges defined as
> > 
> >     Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
> >     Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
> > 
> >     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> >     {
> >         Name (RBUF, ResourceTemplate ()
> >         {
> >             Memory32Fixed (ReadWrite,
> >                 0x60200000,         // Address Base
> >                 0x01DF0000,         // Address Length
> >                 )
> >             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
> >                 0x0000,             // Granularity
> >                 0x0000,             // Range Minimum
> >                 0x0001,             // Range Maximum
> >                 0x0000,             // Translation Offset
> >                 0x0002,             // Length
> >                 ,, )
> >         })
> >         Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
> >     }
> > 
> > The Memory32Fixed holds a host bridge window, but it's not properly
> > defined as a "producer" resource.  Consequently the resource gets
> > removed by kernel, and the BAR allocation fails later on:
> > 
> >     [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
> >     [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> >     [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> >     [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > 
> > This eventually prevents the PCIe NVME drive from being accessible.
> > 
> > Add a quirk for these platforms to avoid the resource being removed.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > We are running into the issue on more devices than just Surface Pro X
> > now, so trying to sort it out with a quirk as suggested by Lorenzo [1].
> 
> One thing I don't like about this application of quirks is that the
> list of affected platforms is likely to grow, which is an ongoing
> burden for users and developers.

It's a very reasonable concern.  I really hope that Qualcomm will start
thinking about Linux support on these machines in the future not too far
away, so that the list will not grow too long.

> Can we have a conversation with Qualcomm about how they *intend* this
> to work?  Linux is probably doing something wrong (interpreting
> something differently than Windows does), and if we could fix that, we
> have a better chance of future platforms working without quirks.

Today Qualcomm only ships and cares about Windows on these machines, but
I believe it will change sooner or later.

Shawn
  

On Thu, Mar 09, 2023 at 10:52:13AM +0800, Shawn Guo wrote:
> + linux-arm-msm and MSM maintainer Bjorn
> 
> On Wed, Mar 08, 2023 at 12:53:10PM -0600, Bjorn Helgaas wrote:
> > On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> > > Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
> > > host bridge windows") introduced a check to remove host bridge register
> > > resources for all arm64 platforms, with the assumption that the PNP0A03
> > > _CRS resources would always be host bridge registers and never as windows
> > > on arm64.
> > > 
> > > The assumption stands true until Qualcomm Snapdragon Windows laptops
> > > emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
> > > resources instead.  For example, the Microsoft Surface Pro X has host
> > > bridges defined as
> > > 
> > >     Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
> > >     Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
> > > 
> > >     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> > >     {
> > >         Name (RBUF, ResourceTemplate ()
> > >         {
> > >             Memory32Fixed (ReadWrite,
> > >                 0x60200000,         // Address Base
> > >                 0x01DF0000,         // Address Length
> > >                 )
> > >             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
> > >                 0x0000,             // Granularity
> > >                 0x0000,             // Range Minimum
> > >                 0x0001,             // Range Maximum
> > >                 0x0000,             // Translation Offset
> > >                 0x0002,             // Length
> > >                 ,, )
> > >         })
> > >         Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
> > >     }
> > > 
> > > The Memory32Fixed holds a host bridge window, but it's not properly
> > > defined as a "producer" resource.  Consequently the resource gets
> > > removed by kernel, and the BAR allocation fails later on:
> > > 
> > >     [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > >     [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > >     [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > >     [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > 
> > > This eventually prevents the PCIe NVME drive from being accessible.
> > > 
> > > Add a quirk for these platforms to avoid the resource being removed.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > ---
> > > We are running into the issue on more devices than just Surface Pro X
> > > now, so trying to sort it out with a quirk as suggested by Lorenzo [1].
> > 
> > One thing I don't like about this application of quirks is that the
> > list of affected platforms is likely to grow, which is an ongoing
> > burden for users and developers.
> 
> It's a very reasonable concern.  I really hope that Qualcomm will start
> thinking about Linux support on these machines in the future not too far
> away, so that the list will not grow too long.
> 
> > Can we have a conversation with Qualcomm about how they *intend* this
> > to work?  Linux is probably doing something wrong (interpreting
> > something differently than Windows does), and if we could fix that, we
> > have a better chance of future platforms working without quirks.
> 
> Today Qualcomm only ships and cares about Windows on these machines, but
> I believe it will change sooner or later.

I don't maintain arch/arm64/kernel/pci.c, but my opinion is that we
should not pursue quirks like this until we've tried really hard to
figure out a generic approach.

Bjorn
  

On Wed, Mar 08, 2023 at 12:53:10PM -0600, Bjorn Helgaas wrote:
> On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> > Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
> > host bridge windows") introduced a check to remove host bridge register
> > resources for all arm64 platforms, with the assumption that the PNP0A03
> > _CRS resources would always be host bridge registers and never as windows
> > on arm64.
> > 
> > The assumption stands true until Qualcomm Snapdragon Windows laptops
> > emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
> > resources instead.  For example, the Microsoft Surface Pro X has host
> > bridges defined as
> > 
> >     Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
> >     Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
> > 
> >     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> >     {
> >         Name (RBUF, ResourceTemplate ()
> >         {
> >             Memory32Fixed (ReadWrite,
> >                 0x60200000,         // Address Base
> >                 0x01DF0000,         // Address Length
> >                 )
> >             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
> >                 0x0000,             // Granularity
> >                 0x0000,             // Range Minimum
> >                 0x0001,             // Range Maximum
> >                 0x0000,             // Translation Offset
> >                 0x0002,             // Length
> >                 ,, )
> >         })
> >         Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
> >     }
> > 
> > The Memory32Fixed holds a host bridge window, but it's not properly
> > defined as a "producer" resource.  Consequently the resource gets
> > removed by kernel, and the BAR allocation fails later on:
> > 
> >     [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
> >     [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> >     [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> >     [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > 
> > This eventually prevents the PCIe NVME drive from being accessible.
> > 
> > Add a quirk for these platforms to avoid the resource being removed.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > We are running into the issue on more devices than just Surface Pro X
> > now, so trying to sort it out with a quirk as suggested by Lorenzo [1].
> 
> One thing I don't like about this application of quirks is that the
> list of affected platforms is likely to grow, which is an ongoing
> burden for users and developers.
> 
> Can we have a conversation with Qualcomm about how they *intend* this
> to work?  Linux is probably doing something wrong (interpreting
> something differently than Windows does), and if we could fix that, we
> have a better chance of future platforms working without quirks.

Catch-22. What if some firmware would add host bridge MMIO register
space (marked as consumer) in the _CRS ? We would end up allocating
BAR regions in there, which is not right, so your commit:

8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from host bridge windows")

is correct and if we revert it we would trigger regressions on some
arm64 platforms for the reason I mention above.

We can look for clarification at ACPI specs level but for firmware
that is out there I am not sure what options we have.

Lorenzo

> > [1] https://lore.kernel.org/all/20210527093200.GA16444@lpieralisi/
> > 
> >  arch/arm64/kernel/pci.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index 2276689b5411..896dbd028b67 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -109,16 +109,42 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> >  	return 0;
> >  }
> >  
> > +#define QCOM_DSDT_QUIRK "Host bridge windows in PNP0A03 _CRS"
> > +
> > +static struct acpi_platform_list qcom_platlist[] = {
> > +	/* Thinkpad X13s */
> > +	{ "LENOVO", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > +	/* Microsoft Surface Pro 9 (5G) and Windows Dev Kit 2023 */
> > +	{ "QCOMM ", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > +	/* Microsoft Surface Pro X */
> > +	{ "QCOMM ", "SDM8180 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > +	{ }
> > +};
> > +
> >  static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
> >  {
> >  	struct resource_entry *entry, *tmp;
> >  	int status;
> > +	int idx;
> >  
> >  	status = acpi_pci_probe_root_resources(ci);
> > +
> > +	/*
> > +	 * Most arm64 platforms that do not run Windows describe host bridge
> > +	 * registers in PNP0A03 _CRS resources, but some like Qualcomm
> > +	 * Snapdragon Windows laptops describe host bridge windows in there.
> > +	 * We do not want to destroy the resources for these platforms.
> > +	 */
> > +	idx = acpi_match_platform_list(qcom_platlist);
> > +	if (idx >= 0)
> > +		goto done;
> > +
> >  	resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
> >  		if (!(entry->res->flags & IORESOURCE_WINDOW))
> >  			resource_list_destroy_entry(entry);
> >  	}
> > +
> > +done:
> >  	return status;
> >  }
> >  
> > -- 
> > 2.17.1
> >
  

On Fri, Mar 10, 2023 at 02:26:55PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Mar 08, 2023 at 12:53:10PM -0600, Bjorn Helgaas wrote:
> > On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> > > Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
> > > host bridge windows") introduced a check to remove host bridge register
> > > resources for all arm64 platforms, with the assumption that the PNP0A03
> > > _CRS resources would always be host bridge registers and never as windows
> > > on arm64.
> > > 
> > > The assumption stands true until Qualcomm Snapdragon Windows laptops
> > > emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
> > > resources instead.  For example, the Microsoft Surface Pro X has host
> > > bridges defined as
> > > 
> > >     Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
> > >     Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
> > > 
> > >     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> > >     {
> > >         Name (RBUF, ResourceTemplate ()
> > >         {
> > >             Memory32Fixed (ReadWrite,
> > >                 0x60200000,         // Address Base
> > >                 0x01DF0000,         // Address Length
> > >                 )
> > >             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
> > >                 0x0000,             // Granularity
> > >                 0x0000,             // Range Minimum
> > >                 0x0001,             // Range Maximum
> > >                 0x0000,             // Translation Offset
> > >                 0x0002,             // Length
> > >                 ,, )
> > >         })
> > >         Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
> > >     }
> > > 
> > > The Memory32Fixed holds a host bridge window, but it's not properly
> > > defined as a "producer" resource.  Consequently the resource gets
> > > removed by kernel, and the BAR allocation fails later on:
> > > 
> > >     [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > >     [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > >     [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > >     [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > 
> > > This eventually prevents the PCIe NVME drive from being accessible.
> > > 
> > > Add a quirk for these platforms to avoid the resource being removed.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > ---
> > > We are running into the issue on more devices than just Surface Pro X
> > > now, so trying to sort it out with a quirk as suggested by Lorenzo [1].
> > 
> > One thing I don't like about this application of quirks is that the
> > list of affected platforms is likely to grow, which is an ongoing
> > burden for users and developers.
> > 
> > Can we have a conversation with Qualcomm about how they *intend* this
> > to work?  Linux is probably doing something wrong (interpreting
> > something differently than Windows does), and if we could fix that, we
> > have a better chance of future platforms working without quirks.
> 
> Catch-22. What if some firmware would add host bridge MMIO register
> space (marked as consumer) in the _CRS ? We would end up allocating
> BAR regions in there, which is not right, so your commit:
> 
> 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from host bridge windows")
> 
> is correct and if we revert it we would trigger regressions on some
> arm64 platforms for the reason I mention above.
> 
> We can look for clarification at ACPI specs level but for firmware
> that is out there I am not sure what options we have.

I don't remember why 8fd4391ee717 exists; I assume there was some
platform that needed it.  I should have included that in the commit
log; mea culpa.

In any event, I assume Windows works on both that platform and the
ones mentioned in this quirk, and I assume Windows doesn't require
platform-specific quirks for something like this.  I admit that's a
lot of assuming, but if Windows can do it, Linux should be able to do
it, too.

> > > +static struct acpi_platform_list qcom_platlist[] = {
> > > +	/* Thinkpad X13s */
> > > +	{ "LENOVO", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > > +	/* Microsoft Surface Pro 9 (5G) and Windows Dev Kit 2023 */
> > > +	{ "QCOMM ", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > > +	/* Microsoft Surface Pro X */
> > > +	{ "QCOMM ", "SDM8180 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
  

On Fri, Mar 10, 2023 at 05:05:39PM -0600, Bjorn Helgaas wrote:
> On Fri, Mar 10, 2023 at 02:26:55PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Mar 08, 2023 at 12:53:10PM -0600, Bjorn Helgaas wrote:
> > > On Mon, Feb 27, 2023 at 10:12:21AM +0800, Shawn Guo wrote:
> > > > Commit 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from
> > > > host bridge windows") introduced a check to remove host bridge register
> > > > resources for all arm64 platforms, with the assumption that the PNP0A03
> > > > _CRS resources would always be host bridge registers and never as windows
> > > > on arm64.
> > > > 
> > > > The assumption stands true until Qualcomm Snapdragon Windows laptops
> > > > emerge.  These laptops describe host bridge windows in PNP0A03 _CRS
> > > > resources instead.  For example, the Microsoft Surface Pro X has host
> > > > bridges defined as
> > > > 
> > > >     Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
> > > >     Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
> > > > 
> > > >     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> > > >     {
> > > >         Name (RBUF, ResourceTemplate ()
> > > >         {
> > > >             Memory32Fixed (ReadWrite,
> > > >                 0x60200000,         // Address Base
> > > >                 0x01DF0000,         // Address Length
> > > >                 )
> > > >             WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
> > > >                 0x0000,             // Granularity
> > > >                 0x0000,             // Range Minimum
> > > >                 0x0001,             // Range Maximum
> > > >                 0x0000,             // Translation Offset
> > > >                 0x0002,             // Length
> > > >                 ,, )
> > > >         })
> > > >         Return (RBUF) /* \_SB_.PCI0._CRS.RBUF */
> > > >     }
> > > > 
> > > > The Memory32Fixed holds a host bridge window, but it's not properly
> > > > defined as a "producer" resource.  Consequently the resource gets
> > > > removed by kernel, and the BAR allocation fails later on:
> > > > 
> > > >     [ 0.150731] pci 0002:00:00.0: BAR 14: no space for [mem size 0x00100000]
> > > >     [ 0.150744] pci 0002:00:00.0: BAR 14: failed to assign [mem size 0x00100000]
> > > >     [ 0.150758] pci 0002:01:00.0: BAR 0: no space for [mem size 0x00004000 64bit]
> > > >     [ 0.150769] pci 0002:01:00.0: BAR 0: failed to assign [mem size 0x00004000 64bit]
> > > > 
> > > > This eventually prevents the PCIe NVME drive from being accessible.
> > > > 
> > > > Add a quirk for these platforms to avoid the resource being removed.
> > > > 
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > ---
> > > > We are running into the issue on more devices than just Surface Pro X
> > > > now, so trying to sort it out with a quirk as suggested by Lorenzo [1].
> > > 
> > > One thing I don't like about this application of quirks is that the
> > > list of affected platforms is likely to grow, which is an ongoing
> > > burden for users and developers.
> > > 
> > > Can we have a conversation with Qualcomm about how they *intend* this
> > > to work?  Linux is probably doing something wrong (interpreting
> > > something differently than Windows does), and if we could fix that, we
> > > have a better chance of future platforms working without quirks.
> > 
> > Catch-22. What if some firmware would add host bridge MMIO register
> > space (marked as consumer) in the _CRS ? We would end up allocating
> > BAR regions in there, which is not right, so your commit:
> > 
> > 8fd4391ee717 ("arm64: PCI: Exclude ACPI "consumer" resources from host bridge windows")
> > 
> > is correct and if we revert it we would trigger regressions on some
> > arm64 platforms for the reason I mention above.
> > 
> > We can look for clarification at ACPI specs level but for firmware
> > that is out there I am not sure what options we have.
> 
> I don't remember why 8fd4391ee717 exists; I assume there was some
> platform that needed it.  I should have included that in the commit
> log; mea culpa.

I believe it is because there were arm64 platforms (early) that added a
consumer descriptor in the host bridge CRS with MMIO registers space in
it (I am not sure I can find the bug report - it has been a while,
remember the issue with non-ECAM config space and where to add the MMIO
resource required to "extend" MCFG config space ? I will never forget
that :)).

> In any event, I assume Windows works on both that platform and the
> ones mentioned in this quirk, and I assume Windows doesn't require
> platform-specific quirks for something like this.

I don't think it can work *without* quirks.

If, for reasons that I don't understand, anyone would add consumer
resources in the host bridge CRS that include at the same time PCI
memory windows and bridge MMIO registers, how can Windows detect which
one is what ? There is no way - at least none I can see.

Most likely, on platforms on which Windows boots, nobody ever added
in FW MMIO register space as a consumer resource in the host bridge
CRS (or Windows has a quirk mechanism for those).

> I admit that's a lot of assuming, but if Windows can do it, Linux
> should be able to do it, too.

I don't think it can do it, would be happy to be wrong.

I can ask, that will not solve anything but at least we know.

Lorenzo

> > > > +static struct acpi_platform_list qcom_platlist[] = {
> > > > +	/* Thinkpad X13s */
> > > > +	{ "LENOVO", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > > > +	/* Microsoft Surface Pro 9 (5G) and Windows Dev Kit 2023 */
> > > > +	{ "QCOMM ", "SDM8280 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },
> > > > +	/* Microsoft Surface Pro X */
> > > > +	{ "QCOMM ", "SDM8180 ", 0, ACPI_SIG_DSDT, all_versions, QCOM_DSDT_QUIRK },