| Message ID | 20190606040919.110655-1-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr_pci: Advertise BAR reallocation capability | expand |
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); >
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); > > >
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);
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); >
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.
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);
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(+)