Message ID | 20211001173358.863017-11-jean-philippe@linaro.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | virtio-iommu: Add ACPI support | expand |
On Fri, 1 Oct 2021, Jean-Philippe Brucker wrote: > The VIOT blob contains the following: > > [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] > [004h 0004 4] Table Length : 00000058 > [008h 0008 1] Revision : 00 > [009h 0009 1] Checksum : 66 > [00Ah 0010 6] Oem ID : "BOCHS " > [010h 0016 8] Oem Table ID : "BXPC " > [018h 0024 4] Oem Revision : 00000001 > [01Ch 0028 4] Asl Compiler ID : "BXPC" > [020h 0032 4] Asl Compiler Revision : 00000001 > > [024h 0036 2] Node count : 0002 > [026h 0038 2] Node offset : 0030 > [028h 0040 8] Reserved : 0000000000000000 > > [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] > [031h 0049 1] Reserved : 00 > [032h 0050 2] Length : 0010 > > [034h 0052 2] PCI Segment : 0000 > [036h 0054 2] PCI BDF number : 0008 > [038h 0056 8] Reserved : 0000000000000000 > > [040h 0064 1] Type : 01 [PCI Range] > [041h 0065 1] Reserved : 00 > [042h 0066 2] Length : 0018 > > [044h 0068 4] Endpoint start : 00000000 > [048h 0072 2] PCI Segment start : 0000 > [04Ah 0074 2] PCI Segment end : 0000 > [04Ch 0076 2] PCI BDF start : 0000 > [04Eh 0078 2] PCI BDF end : 00FF > [050h 0080 2] Output node : 0030 > [052h 0082 6] Reserved : 000000000000 > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Acked-by: Ani Sinha <ani@anisinha.ca> Without looking at the other patches, the disassembly looks good (with latest iasl from upstream git). One suggestion : maybe also add the raw table data as well of length 88. > --- > tests/qtest/bios-tables-test-allowed-diff.h | 1 - > tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes > 2 files changed, 1 deletion(-) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h > index 29b5b1eabc..fa213e4738 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1,4 +1,3 @@ > /* List of comma-separated changed AML files to ignore */ > -"tests/data/acpi/virt/VIOT", > "tests/data/acpi/q35/DSDT.viot", > "tests/data/acpi/q35/VIOT.viot", > diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT > index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d 100644 > GIT binary patch > literal 88 > zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX > I{D-Rq0Q5fy0RR91 > > literal 0 > HcmV?d00001 > > -- > 2.33.0 > >
Hi Jean, On 10/1/21 7:33 PM, Jean-Philippe Brucker wrote: > The VIOT blob contains the following: > > [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] > [004h 0004 4] Table Length : 00000058 > [008h 0008 1] Revision : 00 > [009h 0009 1] Checksum : 66 > [00Ah 0010 6] Oem ID : "BOCHS " > [010h 0016 8] Oem Table ID : "BXPC " > [018h 0024 4] Oem Revision : 00000001 > [01Ch 0028 4] Asl Compiler ID : "BXPC" > [020h 0032 4] Asl Compiler Revision : 00000001 > > [024h 0036 2] Node count : 0002 > [026h 0038 2] Node offset : 0030 > [028h 0040 8] Reserved : 0000000000000000 > > [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] > [031h 0049 1] Reserved : 00 > [032h 0050 2] Length : 0010 > > [034h 0052 2] PCI Segment : 0000 > [036h 0054 2] PCI BDF number : 0008 > [038h 0056 8] Reserved : 0000000000000000 > > [040h 0064 1] Type : 01 [PCI Range] > [041h 0065 1] Reserved : 00 > [042h 0066 2] Length : 0018 > > [044h 0068 4] Endpoint start : 00000000 > [048h 0072 2] PCI Segment start : 0000 > [04Ah 0074 2] PCI Segment end : 0000 > [04Ch 0076 2] PCI BDF start : 0000 > [04Eh 0078 2] PCI BDF end : 00FF > [050h 0080 2] Output node : 0030 > [052h 0082 6] Reserved : 000000000000 I noticed the spec does not clearly say the virtio-iommu-pci BDF does not need to be excluded from the PCI range. Shouldn't it be clarified? Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > tests/qtest/bios-tables-test-allowed-diff.h | 1 - > tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes > 2 files changed, 1 deletion(-) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h > index 29b5b1eabc..fa213e4738 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1,4 +1,3 @@ > /* List of comma-separated changed AML files to ignore */ > -"tests/data/acpi/virt/VIOT", > "tests/data/acpi/q35/DSDT.viot", > "tests/data/acpi/q35/VIOT.viot", > diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT > index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d 100644 > GIT binary patch > literal 88 > zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX > I{D-Rq0Q5fy0RR91 > > literal 0 > HcmV?d00001 >
On Tue, Oct 05, 2021 at 09:38:13PM +0200, Eric Auger wrote: > Hi Jean, > > On 10/1/21 7:33 PM, Jean-Philippe Brucker wrote: > > The VIOT blob contains the following: > > > > [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] > > [004h 0004 4] Table Length : 00000058 > > [008h 0008 1] Revision : 00 > > [009h 0009 1] Checksum : 66 > > [00Ah 0010 6] Oem ID : "BOCHS " > > [010h 0016 8] Oem Table ID : "BXPC " > > [018h 0024 4] Oem Revision : 00000001 > > [01Ch 0028 4] Asl Compiler ID : "BXPC" > > [020h 0032 4] Asl Compiler Revision : 00000001 > > > > [024h 0036 2] Node count : 0002 > > [026h 0038 2] Node offset : 0030 > > [028h 0040 8] Reserved : 0000000000000000 > > > > [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] > > [031h 0049 1] Reserved : 00 > > [032h 0050 2] Length : 0010 > > > > [034h 0052 2] PCI Segment : 0000 > > [036h 0054 2] PCI BDF number : 0008 > > [038h 0056 8] Reserved : 0000000000000000 > > > > [040h 0064 1] Type : 01 [PCI Range] > > [041h 0065 1] Reserved : 00 > > [042h 0066 2] Length : 0018 > > > > [044h 0068 4] Endpoint start : 00000000 > > [048h 0072 2] PCI Segment start : 0000 > > [04Ah 0074 2] PCI Segment end : 0000 > > [04Ch 0076 2] PCI BDF start : 0000 > > [04Eh 0078 2] PCI BDF end : 00FF > > [050h 0080 2] Output node : 0030 > > [052h 0082 6] Reserved : 000000000000 > I noticed the spec does not clearly say the virtio-iommu-pci BDF does > not need to be excluded from the PCI range. > Shouldn't it be clarified? Possibly, but I didn't want to complicate things. As the spec doesn't specify any exception the driver should handle it > > Besides > Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks! Jean
On Tue, Oct 05, 2021 at 03:34:57PM +0530, Ani Sinha wrote: > > > On Fri, 1 Oct 2021, Jean-Philippe Brucker wrote: > > > The VIOT blob contains the following: > > > > [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] > > [004h 0004 4] Table Length : 00000058 > > [008h 0008 1] Revision : 00 > > [009h 0009 1] Checksum : 66 > > [00Ah 0010 6] Oem ID : "BOCHS " > > [010h 0016 8] Oem Table ID : "BXPC " > > [018h 0024 4] Oem Revision : 00000001 > > [01Ch 0028 4] Asl Compiler ID : "BXPC" > > [020h 0032 4] Asl Compiler Revision : 00000001 > > > > [024h 0036 2] Node count : 0002 > > [026h 0038 2] Node offset : 0030 > > [028h 0040 8] Reserved : 0000000000000000 > > > > [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] > > [031h 0049 1] Reserved : 00 > > [032h 0050 2] Length : 0010 > > > > [034h 0052 2] PCI Segment : 0000 > > [036h 0054 2] PCI BDF number : 0008 > > [038h 0056 8] Reserved : 0000000000000000 > > > > [040h 0064 1] Type : 01 [PCI Range] > > [041h 0065 1] Reserved : 00 > > [042h 0066 2] Length : 0018 > > > > [044h 0068 4] Endpoint start : 00000000 > > [048h 0072 2] PCI Segment start : 0000 > > [04Ah 0074 2] PCI Segment end : 0000 > > [04Ch 0076 2] PCI BDF start : 0000 > > [04Eh 0078 2] PCI BDF end : 00FF > > [050h 0080 2] Output node : 0030 > > [052h 0082 6] Reserved : 000000000000 > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Acked-by: Ani Sinha <ani@anisinha.ca> > > Without looking at the other patches, the disassembly looks good (with > latest iasl from upstream git). > One suggestion : maybe also add the raw table data as well of length 88. Hmm, the raw VIOT table is included at the end of the patch (encoded by git in base85). Or did you mean any other data? Thanks, Jean > > > > --- > > tests/qtest/bios-tables-test-allowed-diff.h | 1 - > > tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes > > 2 files changed, 1 deletion(-) > > > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h > > index 29b5b1eabc..fa213e4738 100644 > > --- a/tests/qtest/bios-tables-test-allowed-diff.h > > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > > @@ -1,4 +1,3 @@ > > /* List of comma-separated changed AML files to ignore */ > > -"tests/data/acpi/virt/VIOT", > > "tests/data/acpi/q35/DSDT.viot", > > "tests/data/acpi/q35/VIOT.viot", > > diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT > > index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d 100644 > > GIT binary patch > > literal 88 > > zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX > > I{D-Rq0Q5fy0RR91 > > > > literal 0 > > HcmV?d00001 > > > > -- > > 2.33.0 > > > >
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index 29b5b1eabc..fa213e4738 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1,4 +1,3 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/virt/VIOT", "tests/data/acpi/q35/DSDT.viot", "tests/data/acpi/q35/VIOT.viot",
The VIOT blob contains the following: [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] [004h 0004 4] Table Length : 00000058 [008h 0008 1] Revision : 00 [009h 0009 1] Checksum : 66 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPC " [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4] Asl Compiler Revision : 00000001 [024h 0036 2] Node count : 0002 [026h 0038 2] Node offset : 0030 [028h 0040 8] Reserved : 0000000000000000 [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] [031h 0049 1] Reserved : 00 [032h 0050 2] Length : 0010 [034h 0052 2] PCI Segment : 0000 [036h 0054 2] PCI BDF number : 0008 [038h 0056 8] Reserved : 0000000000000000 [040h 0064 1] Type : 01 [PCI Range] [041h 0065 1] Reserved : 00 [042h 0066 2] Length : 0018 [044h 0068 4] Endpoint start : 00000000 [048h 0072 2] PCI Segment start : 0000 [04Ah 0074 2] PCI Segment end : 0000 [04Ch 0076 2] PCI BDF start : 0000 [04Eh 0078 2] PCI BDF end : 00FF [050h 0080 2] Output node : 0030 [052h 0082 6] Reserved : 000000000000 Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 1 - tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes 2 files changed, 1 deletion(-) diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d 100644 GIT binary patch literal 88 zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX I{D-Rq0Q5fy0RR91 literal 0 HcmV?d00001