On 15.02.2022 16:25, Rahul Singh wrote:
> {read,write}{l,q} function argument is different for ARM and x86.
> ARM {read,wrie}(l,q} function argument is pointer whereas X86
> {read,wrie}(l,q} function argument is address itself.
I'm afraid I don't follow: x86 has
#define readl(x) (*(volatile uint32_t *)(x))
#define readq(x) (*(volatile uint64_t *)(x))
That's no different from Arm64:
#define readl(c) ({ u32 __v = readl_relaxed(c); __iormb(); __v;
})
#define readl_relaxed(c) ({ u32 __v = le32_to_cpu((__force
__le32)__raw_readl(c)); __v; })
static inline u32 __raw_readl(const volatile void __iomem *addr)
The difference is whether the address is expressed as a pointer, or
_may_ also be expressed as unsigned long. IOW the x86 variant is
perfectly fine to be passed e.g. a void * (preferably qualified
appropriately). The conversion from unsigned long to a pointer type
is actually expressed ...
> @@ -170,31 +170,7 @@ bool vpci_msix_read(struct vpci_msix *msix, unsigned
> long addr,
> return true;
>
> if ( VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, VPCI_MSIX_PBA) )
> - {
> - /*
> - * Access to PBA.
> - *
> - * TODO: note that this relies on having the PBA identity mapped to
> the
> - * guest address space. If this changes the address will need to be
> - * translated.
> - */
> - switch ( len )
> - {
> - case 4:
> - *data = readl(addr);
> - break;
> -
> - case 8:
> - *data = readq(addr);
> - break;
> -
> - default:
> - ASSERT_UNREACHABLE();
> - break;
> - }
... in the comment ahead of this switch() (and the assumption is likely
wrong for DomU).
But then, Roger: What "identity mapped" is meant here? Surely not GVA ->
GPA, but rather GPA -> HPA? The address here is a guest physical one,
but read{l,q}() act on (host) virtual addresses. This would have been
easier to notice as wrong if read{l,q}() weren't allowing unsigned long
arguments to be passed to them.
Jan
