Re: [PATCH v15 1/5] arm/vpci: honor access size when returning an error

2024-05-21 Thread Julien Grall

Hi Stewart,

On 17/05/2024 18:06, Stewart Hildebrand wrote:

From: Volodymyr Babchuk 

Guest can try to read config space using different access sizes: 8,
16, 32, 64 bits. We need to take this into account when we are
returning an error back to MMIO handler, otherwise it is possible to
provide more data than requested: i.e. guest issues LDRB instruction
to read one byte, but we are writing 0x in the target
register.

Signed-off-by: Volodymyr Babchuk 
Signed-off-by: Stewart Hildebrand 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



[PATCH v15 1/5] arm/vpci: honor access size when returning an error

2024-05-17 Thread Stewart Hildebrand
From: Volodymyr Babchuk 

Guest can try to read config space using different access sizes: 8,
16, 32, 64 bits. We need to take this into account when we are
returning an error back to MMIO handler, otherwise it is possible to
provide more data than requested: i.e. guest issues LDRB instruction
to read one byte, but we are writing 0x in the target
register.

Signed-off-by: Volodymyr Babchuk 
Signed-off-by: Stewart Hildebrand 
---
v14->v15:
* re-order so this patch comes before ("xen/arm: translate virtual PCI
  bus topology for guests")
* s/access_mask/invalid/
* add U suffix to 1
* s/uint8_t/unsigned int/
* s/uint64_t/register_t/
* although Julien gave an Acked-by on v14, I omitted it due to the
  changes made in v15

v9->10:
* New patch in v10.
---
 xen/arch/arm/vpci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 3bc4bb55082a..b63a356bb4a8 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -29,6 +29,8 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
 {
 struct pci_host_bridge *bridge = p;
 pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
+const unsigned int access_size = (1U << info->dabt.size) * 8;
+const register_t invalid = GENMASK_ULL(access_size - 1, 0);
 /* data is needed to prevent a pointer cast on 32bit */
 unsigned long data;
 
@@ -39,7 +41,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
 return 1;
 }
 
-*r = ~0ul;
+*r = invalid;
 
 return 0;
 }
-- 
2.45.1