On 8/24/21 3:15 PM, Stefan Hajnoczi wrote:
> On Mon, Aug 23, 2021 at 06:41:57PM +0200, 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).
>>
>> The MEMTXPERM_UNRESTRICTED case works as an allow list. Devices
>> using it won't be checked by flatview_access_allowed().
>>
>> 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é <[email protected]>
>> ---
>> 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;
>> + }
>> + 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;
>
> Why bitwise OR?
MemTxResult is not an enum but used as a bitfield.
See access_with_adjusted_size():
MemTxResult r = MEMTX_OK;
...
if (memory_region_big_endian(mr)) {
for (i = 0; i < size; i += access_size) {
r |= access_fn(mr, addr + i, value, access_size,
(size - access_size - i) * 8,
access_mask, attrs);
}
} else {
for (i = 0; i < size; i += access_size) {
r |= access_fn(mr, addr + i, value, access_size, i * 8,
access_mask, attrs);
}
}
return r;
}
and flatview_read_continue() / flatview_write_continue():
for (;;) {
if (!memory_access_is_direct(mr, true)) {
release_lock |= prepare_mmio_access(mr);
l = memory_access_size(mr, l, addr1);
val = ldn_he_p(buf, l);
result |= memory_region_dispatch_write(mr, addr1, val,
size_memop(l),
attrs);
...
return result;
}