Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/29/15 13:37, Marcel Apfelbaum wrote:
> On 11/26/2015 07:01 PM, Laszlo Ersek wrote:
>> Hello Marcel,
>>
>
> [...] if you have ACPI table dumps from within an i440fx
>> SeaBIOS Linux guest, both from before and after your QEMU patches, and
>> those dumps are identical, then that's good evidence against
>> regressions. (I tend to do such acpidump-based comparisons when messing
>> with ACPI builder code.)
>>
>
> Hi,
>
> OK, there are no functional differences between the SSDT before/after,
> however the optimization made in patch "1/3 hw/acpi: merge pxb adjacent
> memory/IO ranges"
> for pxb-pcies works also for pxb, which is a good thing.
>
> SSDT before (only PXB differences) :
> ---
>
> Device (PC0A)
> {
>...
> Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
> {
> ...
> DWordMemory (ResourceProducer, PosDecode, MinFixed,
> MaxFixed, NonCacheable, ReadWrite,
> 0x, // Granularity
> 0xFE80, // Range Minimum
> 0xFE9F, // Range Maximum
> 0x, // Translation Offset
> 0x0020, // Length
> ,, , AddressRangeMemory, TypeStatic)
> DWordMemory (ResourceProducer, PosDecode, MinFixed,
> MaxFixed, NonCacheable, ReadWrite,
> 0x, // Granularity
> 0xFE00, // Range Minimum
> 0xFE7F, // Range Maximum
> 0x, // Translation Offset
> 0x0080, // Length
> ,, , AddressRangeMemory, TypeStatic)
> ...
> })
> }
>
> SSDT after:
>
>
> Device (PC0A)
> {
> ...
> Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
> {
> ...
> DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
> NonCacheable, ReadWrite,
> 0x, // Granularity
> 0xFE00, // Range Minimum
> 0xFE9F, // Range Maximum
> 0x, // Translation Offset
> 0x00A0, // Length
> ,, , AddressRangeMemory, TypeStatic)
>...
> })
> }
>
> As it can be seen, the optimization works also for PXB by merging the
> MEM regions.
>
> Thanks,
> Marcel
>
> [...]
Looks good and makes sense, thanks.
Laszlo
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/26/2015 07:01 PM, Laszlo Ersek wrote:
Hello Marcel,
[...] if you have ACPI table dumps from within an i440fx
SeaBIOS Linux guest, both from before and after your QEMU patches, and
those dumps are identical, then that's good evidence against
regressions. (I tend to do such acpidump-based comparisons when messing
with ACPI builder code.)
Hi,
OK, there are no functional differences between the SSDT before/after,
however the optimization made in patch "1/3 hw/acpi: merge pxb adjacent memory/IO
ranges"
for pxb-pcies works also for pxb, which is a good thing.
SSDT before (only PXB differences) :
---
Device (PC0A)
{
...
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
...
DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
NonCacheable, ReadWrite,
0x, // Granularity
0xFE80, // Range Minimum
0xFE9F, // Range Maximum
0x, // Translation Offset
0x0020, // Length
,, , AddressRangeMemory, TypeStatic)
DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
NonCacheable, ReadWrite,
0x, // Granularity
0xFE00, // Range Minimum
0xFE7F, // Range Maximum
0x, // Translation Offset
0x0080, // Length
,, , AddressRangeMemory, TypeStatic)
...
})
}
SSDT after:
Device (PC0A)
{
...
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
...
DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
NonCacheable, ReadWrite,
0x, // Granularity
0xFE00, // Range Minimum
0xFE9F, // Range Maximum
0x, // Translation Offset
0x00A0, // Length
,, , AddressRangeMemory, TypeStatic)
...
})
}
As it can be seen, the optimization works also for PXB by merging the MEM
regions.
Thanks,
Marcel
[...]
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/27/2015 07:04 PM, Igor Mammedov wrote: On Thu, 26 Nov 2015 20:35:59 +0200 Marcel Apfelbaum wrote: On 11/26/2015 07:01 PM, Laszlo Ersek wrote: Hello Marcel, On 11/26/15 17:00, Marcel Apfelbaum wrote: Note: I took the liberty to CC all the reviewers that took their time and had a look on the previous version, thanks!! The PXB host bridge provides a way to have multiple PCI hierarchies (PCI root buses). This series introduces the pxb-pcie counterpart for PCI Express machines(Currently Q35). This approach works because the Root Complexes are exposed to guest as regular (legacy) opaque PCI host bridges. Tested on Fedora and Windows guests with both Root Ports and PCIe Switches. v2 -> v3: Addressed Eduardo Habkost comments: - Added a bus property to PC machines and use it when querying bus 0. Addressed comments from multiple reviewers (Paolo,Markus,Gerd,Michael) - The issue was the backport compatibility when the PXB changes. - Following all the comments I chose: - Leave the PXB intact as it does the job and all its features (including the internal pci bridge) makes sense. - Add a new device that re-uses all the PXB code but is exposed as a different device to guests. - Once the functionality of the new device diverges we will have no problem to separate the code. I don't think I can productively contribute to the review of this series, but at least I'll try to follow the comments of others. Hi Laszlo, Thank you for your comments. Also, your first patch looks like it touches code that is shared by the i440fx PXB. I think it should cause no change in observable behavior, right? This was the intention. Yes, it should be no change visible to the guest. Should I regression test it with OVMF? If so, that might take... forever. :( I am planning to do this myself, I might ping you if I can't compile/run it. I also think that the new device will work out of the box with Q35 + OVMF. On the other hand, if you have ACPI table dumps from within an i440fx SeaBIOS Linux guest, both from before and after your QEMU patches, and those dumps are identical, then that's good evidence against regressions. (I tend to do such acpidump-based comparisons when messing with ACPI builder code.) This is a good idea, I am going to compare the dumps and get back to you. To do it, you can use patch from my drop_ASL_support_v1 branch: "tests: acpi: print ASL diff in verbose mode" https://github.com/imammedo/qemu/commit/2df59d77151029554a90ce53c059860babadaf30 Hi Igor, Thank you for the pointer, goot to know we have this, but in my case in order to do it I need to change the QEMU command line, re-create the expected asl file, then the expected binary files, and in the end apply my patches and run the test with "V" environment variable. But it may took less than loading the guest. Thanks! Marcel Thanks (and sorry I can't help more ATM) No problem, thank you for your time! Marcel Laszlo v1 -> v2: Addressed Gerd Hoffmann comments: - Added x-enable-internal-bridge compat property to keep the PCI bridge for older machine to avoid breaking migration. Thanks, Marcel Marcel Apfelbaum (3): hw/acpi: merge pxb adjacent memory/IO ranges hw/pxb: introduce pxb-pcie expander for PCIe machines hw/i386: extend pxb query for all PC machines hw/i386/acpi-build.c| 126 +--- hw/i386/pc.c| 2 +- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c| 1 + hw/pci-bridge/pci_expander_bridge.c | 98 +++- include/hw/i386/pc.h| 1 + include/hw/pci/pci.h| 1 + 7 files changed, 163 insertions(+), 67 deletions(-)
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On Thu, 26 Nov 2015 20:35:59 +0200 Marcel Apfelbaum wrote: > On 11/26/2015 07:01 PM, Laszlo Ersek wrote: > > Hello Marcel, > > > > On 11/26/15 17:00, Marcel Apfelbaum wrote: > >> Note: > >> I took the liberty to CC all the reviewers that took their time > >> and had a look on the previous version, thanks!! > >> > >> The PXB host bridge provides a way to have multiple PCI hierarchies (PCI > >> root buses). > >> This series introduces the pxb-pcie counterpart for PCI Express > >> machines(Currently Q35). > >> > >> This approach works because the Root Complexes are exposed to guest as > >> regular > >> (legacy) opaque PCI host bridges. > >> > >> Tested on Fedora and Windows guests with both Root Ports and PCIe Switches. > >> > >> v2 -> v3: > >> Addressed Eduardo Habkost comments: > >> - Added a bus property to PC machines and use it when querying bus 0. > >> Addressed comments from multiple reviewers (Paolo,Markus,Gerd,Michael) > >> - The issue was the backport compatibility when the PXB changes. > >> - Following all the comments I chose: > >> - Leave the PXB intact as it does the job and all its features > >> (including the internal pci bridge) makes sense. > >> - Add a new device that re-uses all the PXB code but is exposed as > >> a different device to guests. > >> - Once the functionality of the new device diverges we will have > >> no problem to separate the code. > > > > > > I don't think I can productively contribute to the review of this > > series, but at least I'll try to follow the comments of others. > > Hi Laszlo, > > Thank you for your comments. > > > > > Also, your first patch looks like it touches code that is shared by the > > i440fx PXB. I think it should cause no change in observable behavior, right? > > This was the intention. Yes, it should be no change visible to the guest. > > > > > Should I regression test it with OVMF? If so, that might take... forever. :( > > I am planning to do this myself, I might ping you if I can't compile/run it. > I also think that the new device will work out of the box with Q35 + OVMF. > > > > > On the other hand, if you have ACPI table dumps from within an i440fx > > SeaBIOS Linux guest, both from before and after your QEMU patches, and > > those dumps are identical, then that's good evidence against > > regressions. (I tend to do such acpidump-based comparisons when messing > > with ACPI builder code.) > > This is a good idea, I am going to compare the dumps and get back to you. To do it, you can use patch from my drop_ASL_support_v1 branch: "tests: acpi: print ASL diff in verbose mode" https://github.com/imammedo/qemu/commit/2df59d77151029554a90ce53c059860babadaf30 > > > > > Thanks (and sorry I can't help more ATM) > > No problem, thank you for your time! > Marcel > > > Laszlo > > > > > >> > >> v1 -> v2: > >> Addressed Gerd Hoffmann comments: > >> - Added x-enable-internal-bridge compat property to keep the PCI > >> bridge for older machine to avoid breaking migration. > >> > >> Thanks, > >> Marcel > >> > >> Marcel Apfelbaum (3): > >>hw/acpi: merge pxb adjacent memory/IO ranges > >>hw/pxb: introduce pxb-pcie expander for PCIe machines > >>hw/i386: extend pxb query for all PC machines > >> > >> hw/i386/acpi-build.c| 126 > >> +--- > >> hw/i386/pc.c| 2 +- > >> hw/i386/pc_piix.c | 1 + > >> hw/i386/pc_q35.c| 1 + > >> hw/pci-bridge/pci_expander_bridge.c | 98 +++- > >> include/hw/i386/pc.h| 1 + > >> include/hw/pci/pci.h| 1 + > >> 7 files changed, 163 insertions(+), 67 deletions(-) > >> > > >
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/26/2015 07:01 PM, Laszlo Ersek wrote: Hello Marcel, On 11/26/15 17:00, Marcel Apfelbaum wrote: Note: I took the liberty to CC all the reviewers that took their time and had a look on the previous version, thanks!! The PXB host bridge provides a way to have multiple PCI hierarchies (PCI root buses). This series introduces the pxb-pcie counterpart for PCI Express machines(Currently Q35). This approach works because the Root Complexes are exposed to guest as regular (legacy) opaque PCI host bridges. Tested on Fedora and Windows guests with both Root Ports and PCIe Switches. v2 -> v3: Addressed Eduardo Habkost comments: - Added a bus property to PC machines and use it when querying bus 0. Addressed comments from multiple reviewers (Paolo,Markus,Gerd,Michael) - The issue was the backport compatibility when the PXB changes. - Following all the comments I chose: - Leave the PXB intact as it does the job and all its features (including the internal pci bridge) makes sense. - Add a new device that re-uses all the PXB code but is exposed as a different device to guests. - Once the functionality of the new device diverges we will have no problem to separate the code. I don't think I can productively contribute to the review of this series, but at least I'll try to follow the comments of others. Hi Laszlo, Thank you for your comments. Also, your first patch looks like it touches code that is shared by the i440fx PXB. I think it should cause no change in observable behavior, right? This was the intention. Yes, it should be no change visible to the guest. Should I regression test it with OVMF? If so, that might take... forever. :( I am planning to do this myself, I might ping you if I can't compile/run it. I also think that the new device will work out of the box with Q35 + OVMF. On the other hand, if you have ACPI table dumps from within an i440fx SeaBIOS Linux guest, both from before and after your QEMU patches, and those dumps are identical, then that's good evidence against regressions. (I tend to do such acpidump-based comparisons when messing with ACPI builder code.) This is a good idea, I am going to compare the dumps and get back to you. Thanks (and sorry I can't help more ATM) No problem, thank you for your time! Marcel Laszlo v1 -> v2: Addressed Gerd Hoffmann comments: - Added x-enable-internal-bridge compat property to keep the PCI bridge for older machine to avoid breaking migration. Thanks, Marcel Marcel Apfelbaum (3): hw/acpi: merge pxb adjacent memory/IO ranges hw/pxb: introduce pxb-pcie expander for PCIe machines hw/i386: extend pxb query for all PC machines hw/i386/acpi-build.c| 126 +--- hw/i386/pc.c| 2 +- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c| 1 + hw/pci-bridge/pci_expander_bridge.c | 98 +++- include/hw/i386/pc.h| 1 + include/hw/pci/pci.h| 1 + 7 files changed, 163 insertions(+), 67 deletions(-)
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
Hello Marcel, On 11/26/15 17:00, Marcel Apfelbaum wrote: > Note: > I took the liberty to CC all the reviewers that took their time > and had a look on the previous version, thanks!! > > The PXB host bridge provides a way to have multiple PCI hierarchies (PCI root > buses). > This series introduces the pxb-pcie counterpart for PCI Express > machines(Currently Q35). > > This approach works because the Root Complexes are exposed to guest as regular > (legacy) opaque PCI host bridges. > > Tested on Fedora and Windows guests with both Root Ports and PCIe Switches. > > v2 -> v3: > Addressed Eduardo Habkost comments: > - Added a bus property to PC machines and use it when querying bus 0. > Addressed comments from multiple reviewers (Paolo,Markus,Gerd,Michael) > - The issue was the backport compatibility when the PXB changes. > - Following all the comments I chose: >- Leave the PXB intact as it does the job and all its features > (including the internal pci bridge) makes sense. >- Add a new device that re-uses all the PXB code but is exposed as > a different device to guests. >- Once the functionality of the new device diverges we will have > no problem to separate the code. I don't think I can productively contribute to the review of this series, but at least I'll try to follow the comments of others. Also, your first patch looks like it touches code that is shared by the i440fx PXB. I think it should cause no change in observable behavior, right? Should I regression test it with OVMF? If so, that might take... forever. :( On the other hand, if you have ACPI table dumps from within an i440fx SeaBIOS Linux guest, both from before and after your QEMU patches, and those dumps are identical, then that's good evidence against regressions. (I tend to do such acpidump-based comparisons when messing with ACPI builder code.) Thanks (and sorry I can't help more ATM) Laszlo > > v1 -> v2: > Addressed Gerd Hoffmann comments: > - Added x-enable-internal-bridge compat property to keep the PCI >bridge for older machine to avoid breaking migration. > > Thanks, > Marcel > > Marcel Apfelbaum (3): > hw/acpi: merge pxb adjacent memory/IO ranges > hw/pxb: introduce pxb-pcie expander for PCIe machines > hw/i386: extend pxb query for all PC machines > > hw/i386/acpi-build.c| 126 > +--- > hw/i386/pc.c| 2 +- > hw/i386/pc_piix.c | 1 + > hw/i386/pc_q35.c| 1 + > hw/pci-bridge/pci_expander_bridge.c | 98 +++- > include/hw/i386/pc.h| 1 + > include/hw/pci/pci.h| 1 + > 7 files changed, 163 insertions(+), 67 deletions(-) >
