[v3] xen/arm: pci: fix check in pci_check_bar()

[PATCH v3] xen/arm: pci: fix check in pci_check_bar()

Posted by Stewart Hildebrand 9 months, 2 weeks ago

When mapping BARs for vPCI, it's valid for a BAR mfn_t start to equal the BAR
mfn_t end (i.e. start == end) since end is inclusive. However, pci_check_bar()
currently returns false in this case, which results in Xen not mapping the BAR
in the guest 2nd stage page tables. In this example boot log, Linux has mapped
the BARs in the 1st stage, but since Xen did not map them in the 2nd stage,
Linux encounters a data abort and panics:

[    2.593300] pci 0000:00:00.0: BAR 0: assigned [mem 0x50008000-0x50008fff]
[    2.593682] pci 0000:00:00.0: BAR 2: assigned [mem 0x50009000-0x50009fff]
[    2.594066] pci 0000:00:00.0: BAR 4: assigned [mem 0x5000a000-0x5000afff]
...
[    2.810502] virtio-pci 0000:00:00.0: enabling device (0000 -> 0002)
(XEN) 0000:00:00.0: not mapping BAR [50008, 50008] invalid position
(XEN) 0000:00:00.0: not mapping BAR [50009, 50009] invalid position
(XEN) 0000:00:00.0: not mapping BAR [5000a, 5000a] invalid position
[    2.817502] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy driver
[    2.817853] virtio-pci 0000:00:00.0: enabling bus mastering
(XEN) arch/arm/traps.c:1992:d0v0 HSR=0x00000093010045 pc=0xffff8000089507d4 gva=0xffff80000c46d012 gpa=0x00000050008012
[    2.818397] Unable to handle kernel ttbr address size fault at virtual address ffff80000c46d012
...

Adjust the end physical address e to account for the full page when converting
from mfn, at which point s and e cannot be equal, so drop the equality check in
the condition.

Note that adjusting e to account for the full page also increases the accuracy
of the subsequent is_bar_valid check.

Fixes: cc80e2bab0d0 ("xen/pci: replace call to is_memory_hole to pci_check_bar")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
v2->v3:
* re-word commit message slightly
* add Roger's R-b

v1->v2:
* adjust e to include full page (e is still inclusive)
* add comment at the top of the function to document that end is inclusive
---
 xen/arch/arm/pci/pci-host-common.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
index 7cdfc89e5211..c0faf0f43675 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -393,20 +393,24 @@ static int is_bar_valid(const struct dt_device_node *dev,
     return 0;
 }
 
-/* TODO: Revisit this function when ACPI PCI passthrough support is added. */
+/*
+ * The MFN range [start, end] is inclusive.
+ *
+ * TODO: Revisit this function when ACPI PCI passthrough support is added.
+ */
 bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end)
 {
     int ret;
     const struct dt_device_node *dt_node;
     paddr_t s = mfn_to_maddr(start);
-    paddr_t e = mfn_to_maddr(end);
+    paddr_t e = mfn_to_maddr(mfn_add(end, 1)) - 1; /* inclusive */
     struct pdev_bar_check bar_data =  {
         .start = s,
         .end = e,
         .is_valid = false
     };
 
-    if ( s >= e )
+    if ( s > e )
         return false;
 
     dt_node = pci_find_host_bridge_node(pdev);

base-commit: 0a834e953b01ec25c412369d7a5b8b57d340ac60
-- 
2.41.0

Re: [PATCH v3] xen/arm: pci: fix check in pci_check_bar()

Posted by Rahul Singh 9 months, 2 weeks ago

Hi Stewart,

> On 12 Jul 2023, at 2:52 pm, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
> 
> When mapping BARs for vPCI, it's valid for a BAR mfn_t start to equal the BAR
> mfn_t end (i.e. start == end) since end is inclusive. However, pci_check_bar()
> currently returns false in this case, which results in Xen not mapping the BAR
> in the guest 2nd stage page tables. In this example boot log, Linux has mapped
> the BARs in the 1st stage, but since Xen did not map them in the 2nd stage,
> Linux encounters a data abort and panics:
> 
> [    2.593300] pci 0000:00:00.0: BAR 0: assigned [mem 0x50008000-0x50008fff]
> [    2.593682] pci 0000:00:00.0: BAR 2: assigned [mem 0x50009000-0x50009fff]
> [    2.594066] pci 0000:00:00.0: BAR 4: assigned [mem 0x5000a000-0x5000afff]
> ...
> [    2.810502] virtio-pci 0000:00:00.0: enabling device (0000 -> 0002)
> (XEN) 0000:00:00.0: not mapping BAR [50008, 50008] invalid position
> (XEN) 0000:00:00.0: not mapping BAR [50009, 50009] invalid position
> (XEN) 0000:00:00.0: not mapping BAR [5000a, 5000a] invalid position
> [    2.817502] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy driver
> [    2.817853] virtio-pci 0000:00:00.0: enabling bus mastering
> (XEN) arch/arm/traps.c:1992:d0v0 HSR=0x00000093010045 pc=0xffff8000089507d4 gva=0xffff80000c46d012 gpa=0x00000050008012
> [    2.818397] Unable to handle kernel ttbr address size fault at virtual address ffff80000c46d012
> ...
> 
> Adjust the end physical address e to account for the full page when converting
> from mfn, at which point s and e cannot be equal, so drop the equality check in
> the condition.
> 
> Note that adjusting e to account for the full page also increases the accuracy
> of the subsequent is_bar_valid check.
> 
> Fixes: cc80e2bab0d0 ("xen/pci: replace call to is_memory_hole to pci_check_bar")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I tested the patch on N1SDP board everything works.

Reviewed-by: Rahul Singh <rahul.singh@arm.com>
Tested-by: Rahul Singh <rahul.singh@arm.com>

Regards,
Rahul

Re: [PATCH v3] xen/arm: pci: fix check in pci_check_bar()

Posted by Julien Grall 9 months, 2 weeks ago


On 12/07/2023 18:04, Rahul Singh wrote:
> Hi Stewart,
> 
>> On 12 Jul 2023, at 2:52 pm, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
>>
>> When mapping BARs for vPCI, it's valid for a BAR mfn_t start to equal the BAR
>> mfn_t end (i.e. start == end) since end is inclusive. However, pci_check_bar()
>> currently returns false in this case, which results in Xen not mapping the BAR
>> in the guest 2nd stage page tables. In this example boot log, Linux has mapped
>> the BARs in the 1st stage, but since Xen did not map them in the 2nd stage,
>> Linux encounters a data abort and panics:
>>
>> [    2.593300] pci 0000:00:00.0: BAR 0: assigned [mem 0x50008000-0x50008fff]
>> [    2.593682] pci 0000:00:00.0: BAR 2: assigned [mem 0x50009000-0x50009fff]
>> [    2.594066] pci 0000:00:00.0: BAR 4: assigned [mem 0x5000a000-0x5000afff]
>> ...
>> [    2.810502] virtio-pci 0000:00:00.0: enabling device (0000 -> 0002)
>> (XEN) 0000:00:00.0: not mapping BAR [50008, 50008] invalid position
>> (XEN) 0000:00:00.0: not mapping BAR [50009, 50009] invalid position
>> (XEN) 0000:00:00.0: not mapping BAR [5000a, 5000a] invalid position
>> [    2.817502] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy driver
>> [    2.817853] virtio-pci 0000:00:00.0: enabling bus mastering
>> (XEN) arch/arm/traps.c:1992:d0v0 HSR=0x00000093010045 pc=0xffff8000089507d4 gva=0xffff80000c46d012 gpa=0x00000050008012
>> [    2.818397] Unable to handle kernel ttbr address size fault at virtual address ffff80000c46d012
>> ...
>>
>> Adjust the end physical address e to account for the full page when converting
>> from mfn, at which point s and e cannot be equal, so drop the equality check in
>> the condition.
>>
>> Note that adjusting e to account for the full page also increases the accuracy
>> of the subsequent is_bar_valid check.
>>
>> Fixes: cc80e2bab0d0 ("xen/pci: replace call to is_memory_hole to pci_check_bar")
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> I tested the patch on N1SDP board everything works.
> 
> Reviewed-by: Rahul Singh <rahul.singh@arm.com>
> Tested-by: Rahul Singh <rahul.singh@arm.com>

Acked-by: Julien Grall <jgrall@amazon.com>

> 
> Regards,
> Rahul

-- 
Julien Grall

Re: [PATCH v3] xen/arm: pci: fix check in pci_check_bar()

Posted by Julien Grall 9 months, 2 weeks ago


On 13/07/2023 19:18, Julien Grall wrote:
> 
> 
> On 12/07/2023 18:04, Rahul Singh wrote:
>> Hi Stewart,
>>
>>> On 12 Jul 2023, at 2:52 pm, Stewart Hildebrand 
>>> <Stewart.Hildebrand@amd.com> wrote:
>>>
>>> When mapping BARs for vPCI, it's valid for a BAR mfn_t start to equal 
>>> the BAR
>>> mfn_t end (i.e. start == end) since end is inclusive. However, 
>>> pci_check_bar()
>>> currently returns false in this case, which results in Xen not 
>>> mapping the BAR
>>> in the guest 2nd stage page tables. In this example boot log, Linux 
>>> has mapped
>>> the BARs in the 1st stage, but since Xen did not map them in the 2nd 
>>> stage,
>>> Linux encounters a data abort and panics:
>>>
>>> [    2.593300] pci 0000:00:00.0: BAR 0: assigned [mem 
>>> 0x50008000-0x50008fff]
>>> [    2.593682] pci 0000:00:00.0: BAR 2: assigned [mem 
>>> 0x50009000-0x50009fff]
>>> [    2.594066] pci 0000:00:00.0: BAR 4: assigned [mem 
>>> 0x5000a000-0x5000afff]
>>> ...
>>> [    2.810502] virtio-pci 0000:00:00.0: enabling device (0000 -> 0002)
>>> (XEN) 0000:00:00.0: not mapping BAR [50008, 50008] invalid position
>>> (XEN) 0000:00:00.0: not mapping BAR [50009, 50009] invalid position
>>> (XEN) 0000:00:00.0: not mapping BAR [5000a, 5000a] invalid position
>>> [    2.817502] virtio-pci 0000:00:00.0: virtio_pci: leaving for 
>>> legacy driver
>>> [    2.817853] virtio-pci 0000:00:00.0: enabling bus mastering
>>> (XEN) arch/arm/traps.c:1992:d0v0 HSR=0x00000093010045 
>>> pc=0xffff8000089507d4 gva=0xffff80000c46d012 gpa=0x00000050008012
>>> [    2.818397] Unable to handle kernel ttbr address size fault at 
>>> virtual address ffff80000c46d012
>>> ...
>>>
>>> Adjust the end physical address e to account for the full page when 
>>> converting
>>> from mfn, at which point s and e cannot be equal, so drop the 
>>> equality check in
>>> the condition.
>>>
>>> Note that adjusting e to account for the full page also increases the 
>>> accuracy
>>> of the subsequent is_bar_valid check.
>>>
>>> Fixes: cc80e2bab0d0 ("xen/pci: replace call to is_memory_hole to 
>>> pci_check_bar")
>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> I tested the patch on N1SDP board everything works.
>>
>> Reviewed-by: Rahul Singh <rahul.singh@arm.com>
>> Tested-by: Rahul Singh <rahul.singh@arm.com>
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

And committed.

Cheers,

-- 
Julien Grall

Re: [PATCH v3] xen/arm: pci: fix check in pci_check_bar()

Posted by Rahul Singh 9 months, 2 weeks ago

Hi Stewart,

> On 12 Jul 2023, at 2:52 pm, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
> 
> When mapping BARs for vPCI, it's valid for a BAR mfn_t start to equal the BAR
> mfn_t end (i.e. start == end) since end is inclusive. However, pci_check_bar()
> currently returns false in this case, which results in Xen not mapping the BAR
> in the guest 2nd stage page tables. In this example boot log, Linux has mapped
> the BARs in the 1st stage, but since Xen did not map them in the 2nd stage,
> Linux encounters a data abort and panics:
> 
> [    2.593300] pci 0000:00:00.0: BAR 0: assigned [mem 0x50008000-0x50008fff]
> [    2.593682] pci 0000:00:00.0: BAR 2: assigned [mem 0x50009000-0x50009fff]
> [    2.594066] pci 0000:00:00.0: BAR 4: assigned [mem 0x5000a000-0x5000afff]
> ...
> [    2.810502] virtio-pci 0000:00:00.0: enabling device (0000 -> 0002)
> (XEN) 0000:00:00.0: not mapping BAR [50008, 50008] invalid position
> (XEN) 0000:00:00.0: not mapping BAR [50009, 50009] invalid position
> (XEN) 0000:00:00.0: not mapping BAR [5000a, 5000a] invalid position
> [    2.817502] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy driver
> [    2.817853] virtio-pci 0000:00:00.0: enabling bus mastering
> (XEN) arch/arm/traps.c:1992:d0v0 HSR=0x00000093010045 pc=0xffff8000089507d4 gva=0xffff80000c46d012 gpa=0x00000050008012
> [    2.818397] Unable to handle kernel ttbr address size fault at virtual address ffff80000c46d012
> ...
> 
> Adjust the end physical address e to account for the full page when converting
> from mfn, at which point s and e cannot be equal, so drop the equality check in
> the condition.
> 
> Note that adjusting e to account for the full page also increases the accuracy
> of the subsequent is_bar_valid check.
> 
> Fixes: cc80e2bab0d0 ("xen/pci: replace call to is_memory_hole to pci_check_bar")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I tested the patch on N1SDP board everything works.

Reviewed-by: Rahul Singh <rahul.singh@arm.com>
Tested-by: Rahul Singh <rahul.singh@arm.com>

Regards,
Rahul