On 23.08.21 18:41, Philippe Mathieu-Daudé wrote:
Check bus permission in flatview_access_allowed() before
running any bus transaction.
There is not change for the default case (MEMTXPERM_UNSPECIFIED).
s/not/no/
The MEMTXPERM_UNRESTRICTED case works as an allow list. Devices
using it won't be checked by flatview_access_allowed().
Well, and MEMTXPERM_UNSPECIFIED. Another indication that the split
should better be avoided.
The only deny list equivalent is MEMTXPERM_RAM_DEVICE: devices
using this flag will reject transactions and set the optional
MemTxResult to MEMTX_BUS_ERROR.
Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
---
softmmu/physmem.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 0d31a2f4199..329542dba75 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2772,7 +2772,22 @@ static inline bool flatview_access_allowed(MemoryRegion
*mr, MemTxAttrs attrs,
hwaddr addr, hwaddr len,
MemTxResult *result)
{
- return true;
+ if (unlikely(attrs.bus_perm == MEMTXPERM_RAM_DEVICE)) {
+ if (memory_region_is_ram(mr) || memory_region_is_ram_device(mr)) {
+ return true;
+ }
I'm a bit confused why it's called MEMTXPERM_RAM_DEVICE, yet we also
allow access to !memory_region_is_ram_device(mr).
Can we find a more expressive name?
Also, I wonder if we'd actually want to have "flags" instead of pure
values. Like having
#define MEMTXPERM_RAM 1
#define MEMTXPERM_RAM_DEVICE 2
and map them cleanly to the two similar, but different types of memory
backends.
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid access to non-RAM device at "
+ "addr 0x%" HWADDR_PRIX ", size %" HWADDR_PRIu ", "
+ "region '%s'\n", addr, len, memory_region_name(mr));
+ if (result) {
+ *result |= MEMTX_BUS_ERROR;
+ }
+ return false;
+ } else {
+ /* MEMTXPERM_UNRESTRICTED and MEMTXPERM_UNSPECIFIED cases */
+ return true;
+ }
}
/* Called within RCU critical section. */
Do we have any target user examples / prototypes?
--
Thanks,
David / dhildenb
