[Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[Alexey Kardashevskiy]

The pseries guests do not normally allocate PCI resouces and rely on
the system firmware doing so. Furthermore at least at some point in
the past the pseries guests won't even be allowed to change BARs, probably
it is still the case for phyp. So since the initial commit we have [1]
which prevents resource reallocation.

This is not a problem until we want specific BAR alignments, for example,
PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
does not allocate BARs, the guest does this instead and does not align
BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
PCI_PROBE_ONLY makes Linux ignore alignment requests.

ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
This makes use of the dial to advertise to the guest that we can handle
BAR reassignments.

We do not remove the flag from [1] as pseries guests are still supported
under phyp so having that removed may cause problems.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
[2] https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6dd8aaac3340..84d16f9edaca 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1209,6 +1209,9 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
         _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
     }
 
+    /* We can deal with BAR reallocation just fine, advertise it to the guest */
+    _FDT(fdt_setprop_cell(fdt, chosen, "linux,pci-probe-only", 0));
+
     spapr_dt_ov5_platform_support(spapr, fdt, chosen);
 
     g_free(stdout_path);
-- 
2.17.1

Re: [Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[Alexey Kardashevskiy]

I changed my handy scripts for posting patches and the subject line
broke, do I need to repost? It made it to the patchworks though.



On 06/06/2019 14:09, Alexey Kardashevskiy wrote:
> The pseries guests do not normally allocate PCI resouces and rely on
> the system firmware doing so. Furthermore at least at some point in
> the past the pseries guests won't even be allowed to change BARs, probably
> it is still the case for phyp. So since the initial commit we have [1]
> which prevents resource reallocation.
> 
> This is not a problem until we want specific BAR alignments, for example,
> PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
> the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
> does not allocate BARs, the guest does this instead and does not align
> BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
> PCI_PROBE_ONLY makes Linux ignore alignment requests.
> 
> ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
> This makes use of the dial to advertise to the guest that we can handle
> BAR reassignments.
> 
> We do not remove the flag from [1] as pseries guests are still supported
> under phyp so having that removed may cause problems.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
> [2] https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6dd8aaac3340..84d16f9edaca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1209,6 +1209,9 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>          _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
>      }
>  
> +    /* We can deal with BAR reallocation just fine, advertise it to the guest */
> +    _FDT(fdt_setprop_cell(fdt, chosen, "linux,pci-probe-only", 0));
> +
>      spapr_dt_ov5_platform_support(spapr, fdt, chosen);
>  
>      g_free(stdout_path);
> 

-- 
Alexey

Re: [Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2786 bytes --]

On Thu, Jun 06, 2019 at 02:13:20PM +1000, Alexey Kardashevskiy wrote:
> I changed my handy scripts for posting patches and the subject line
> broke, do I need to repost? It made it to the patchworks though.

No, that's fine I've seen it and will look at it when I have the chance.

> 
> 
> 
> On 06/06/2019 14:09, Alexey Kardashevskiy wrote:
> > The pseries guests do not normally allocate PCI resouces and rely on
> > the system firmware doing so. Furthermore at least at some point in
> > the past the pseries guests won't even be allowed to change BARs, probably
> > it is still the case for phyp. So since the initial commit we have [1]
> > which prevents resource reallocation.
> > 
> > This is not a problem until we want specific BAR alignments, for example,
> > PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
> > the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
> > does not allocate BARs, the guest does this instead and does not align
> > BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
> > PCI_PROBE_ONLY makes Linux ignore alignment requests.
> > 
> > ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
> > This makes use of the dial to advertise to the guest that we can handle
> > BAR reassignments.
> > 
> > We do not remove the flag from [1] as pseries guests are still supported
> > under phyp so having that removed may cause problems.
> > 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
> > [2] https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
> > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >  hw/ppc/spapr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 6dd8aaac3340..84d16f9edaca 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1209,6 +1209,9 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
> >          _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
> >      }
> >  
> > +    /* We can deal with BAR reallocation just fine, advertise it to the guest */
> > +    _FDT(fdt_setprop_cell(fdt, chosen, "linux,pci-probe-only", 0));
> > +
> >      spapr_dt_ov5_platform_support(spapr, fdt, chosen);
> >  
> >      g_free(stdout_path);
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]

On Thu, Jun 06, 2019 at 02:09:19PM +1000, Alexey Kardashevskiy wrote:
> The pseries guests do not normally allocate PCI resouces and rely on
> the system firmware doing so. Furthermore at least at some point in
> the past the pseries guests won't even be allowed to change BARs, probably
> it is still the case for phyp. So since the initial commit we have [1]
> which prevents resource reallocation.
> 
> This is not a problem until we want specific BAR alignments, for example,
> PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
> the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
> does not allocate BARs, the guest does this instead and does not align
> BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
> PCI_PROBE_ONLY makes Linux ignore alignment requests.
> 
> ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
> This makes use of the dial to advertise to the guest that we can handle
> BAR reassignments.
> 
> We do not remove the flag from [1] as pseries guests are still supported
> under phyp so having that removed may cause problems.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
> [2] https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Changing a guest visible property, that could have a big effect on how
the guest behaves, without a machine version change seems... unwise.

> ---
>  hw/ppc/spapr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6dd8aaac3340..84d16f9edaca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1209,6 +1209,9 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>          _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
>      }
>  
> +    /* We can deal with BAR reallocation just fine, advertise it to the guest */
> +    _FDT(fdt_setprop_cell(fdt, chosen, "linux,pci-probe-only", 0));
> +
>      spapr_dt_ov5_platform_support(spapr, fdt, chosen);
>  
>      g_free(stdout_path);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[Alexey Kardashevskiy]

On 12/06/2019 16:11, David Gibson wrote:
> On Thu, Jun 06, 2019 at 02:09:19PM +1000, Alexey Kardashevskiy wrote:
>> The pseries guests do not normally allocate PCI resouces and rely on
>> the system firmware doing so. Furthermore at least at some point in
>> the past the pseries guests won't even be allowed to change BARs, probably
>> it is still the case for phyp. So since the initial commit we have [1]
>> which prevents resource reallocation.
>>
>> This is not a problem until we want specific BAR alignments, for example,
>> PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
>> the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
>> does not allocate BARs, the guest does this instead and does not align
>> BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
>> PCI_PROBE_ONLY makes Linux ignore alignment requests.
>>
>> ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
>> This makes use of the dial to advertise to the guest that we can handle
>> BAR reassignments.
>>
>> We do not remove the flag from [1] as pseries guests are still supported
>> under phyp so having that removed may cause problems.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
>> [2] https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Changing a guest visible property, that could have a big effect on how
> the guest behaves, without a machine version change seems... unwise.


As a general rule - sure, not good. In this particular case QEMU has
always been able to cope with BAR reallocations. What could probably
make sense is having it as a machine option (pci-probe-only=off by
default) in case if we find some old kernel which cannot handle
"linux,pci-probe-only" but I seriously doubt we'll find such a broken
kernel - I do remove the probe-only switch from guest kernels on a
regular basis last 7 or so years when debugging.


> 
>> ---
>>  hw/ppc/spapr.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 6dd8aaac3340..84d16f9edaca 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1209,6 +1209,9 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>>          _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
>>      }
>>  
>> +    /* We can deal with BAR reallocation just fine, advertise it to the guest */
>> +    _FDT(fdt_setprop_cell(fdt, chosen, "linux,pci-probe-only", 0));
>> +
>>      spapr_dt_ov5_platform_support(spapr, fdt, chosen);
>>  
>>      g_free(stdout_path);
> 

-- 
Alexey

Re: [Qemu-devel] spapr_pci: Advertise BAR reallocation capability

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2889 bytes --]

On Thu, Jun 13, 2019 at 11:37:45AM +1000, Alexey Kardashevskiy wrote:
> 
> 
> On 12/06/2019 16:11, David Gibson wrote:
> > On Thu, Jun 06, 2019 at 02:09:19PM +1000, Alexey Kardashevskiy wrote:
> >> The pseries guests do not normally allocate PCI resouces and rely on
> >> the system firmware doing so. Furthermore at least at some point in
> >> the past the pseries guests won't even be allowed to change BARs, probably
> >> it is still the case for phyp. So since the initial commit we have [1]
> >> which prevents resource reallocation.
> >>
> >> This is not a problem until we want specific BAR alignments, for example,
> >> PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
> >> the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
> >> does not allocate BARs, the guest does this instead and does not align
> >> BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
> >> PCI_PROBE_ONLY makes Linux ignore alignment requests.
> >>
> >> ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
> >> This makes use of the dial to advertise to the guest that we can handle
> >> BAR reassignments.
> >>
> >> We do not remove the flag from [1] as pseries guests are still supported
> >> under phyp so having that removed may cause problems.
> >>
> >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
> >> [2] https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
> >> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > 
> > Changing a guest visible property, that could have a big effect on how
> > the guest behaves, without a machine version change seems... unwise.
> 
> 
> As a general rule - sure, not good. In this particular case QEMU has
> always been able to cope with BAR reallocations.

That's not really the point.  What I'm worried about is some old
kernel version running on a guest in the wild having a bug here and
the supposedly compatible qemu change breaking it.

> What could probably
> make sense is having it as a machine option (pci-probe-only=off by
> default) in case if we find some old kernel which cannot handle
> "linux,pci-probe-only" but I seriously doubt we'll find such a broken
> kernel - I do remove the probe-only switch from guest kernels on a
> regular basis last 7 or so years when debugging.

Yeah, doing it when debugging isn't really the same as exercising it
in production environments.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]