Hi,
On 03/09/2024 14:19, Andrew Cooper wrote:
On 03/09/2024 11:30 am, Michal Orzel wrote:
On 02/09/2024 12:03, Andrew Cooper wrote:
These are all loops over a scalar value, and don't need to call general bitop
helpers behind the scenes.
No functional change.
Signed-off-by: Andrew Cooper <[email protected]>
---
CC: Stefano Stabellini <[email protected]>
CC: Julien Grall <[email protected]>
CC: Volodymyr Babchuk <[email protected]>
CC: Bertrand Marquis <[email protected]>
CC: Michal Orzel <[email protected]>
Slighly RFC. It's unclear whether len is the size of the register, or the
size of the access. For sub-GPR accesses, won't the upper bits be clear
anyway? If so, this can be simplified further.
See dispatch_mmio_write(). "len" refers to access size (i.e. 1/4/8 bytes). Each
register is defined
with a region access i.e. VGIC_ACCESS_32bit that is compared with the actual
access. In the current code
there is no register with 8B access. If there is a mismatch, there will be a
data abort injected.
Also, the top 32-bits are not cleared anywhere, so I don't think we can drop
masking. @Julien?
That's correct, there are no masking in the I/O dispatch helpers.
Ok, so it is necessary right now to have the clamping logic in every
callback.
However, given that the size is validated before dispatching, clamping
once in dispatch_mmio_write() would save a lot of repeated code in the
callbacks.
i.e., I think this:
diff --git a/xen/arch/arm/vgic/vgic-mmio.c b/xen/arch/arm/vgic/vgic-mmio.c
index bd4caf7fc326..e8b9805a0b2c 100644
--- a/xen/arch/arm/vgic/vgic-mmio.c
+++ b/xen/arch/arm/vgic/vgic-mmio.c
@@ -590,6 +590,9 @@ static int dispatch_mmio_write(struct vcpu *vcpu,
mmio_info_t *info,
if ( !region )
return 0;
+ if ( len < sizeof(data) )
+ data &= ~((1UL << (len * 8)) - 1);
+
I think it would make sense to move the masking one level higher in
handle_write() (arch/arm/io.c). So this would cover all the emulation
helpers.
Cheers,
--
Julien Grall
