On 8/24/21 15:08, Stefan Hajnoczi wrote: > On Mon, Aug 23, 2021 at 06:41:55PM +0200, Philippe Mathieu-Daudé wrote: >> Add the 'direct_access' bit to the memory attributes to restrict >> bus master access to ROM/RAM. >> Have read/write accessors return MEMTX_BUS_ERROR if an access is >> restricted and the region is not ROM/RAM ('direct'). >> Add corresponding trace events. >> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> include/exec/memattrs.h | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h >> index 95f2d20d55b..7a94ee75a88 100644 >> --- a/include/exec/memattrs.h >> +++ b/include/exec/memattrs.h >> @@ -14,6 +14,13 @@ >> #ifndef MEMATTRS_H >> #define MEMATTRS_H >> >> +/* Permission to restrict bus memory accesses. See MemTxAttrs::bus_perm */ >> +enum { >> + MEMTXPERM_UNSPECIFIED = 0, >> + MEMTXPERM_UNRESTRICTED = 1, >> + MEMTXPERM_RAM_DEVICE = 2, >> +}; >> + >> /* Every memory transaction has associated with it a set of >> * attributes. Some of these are generic (such as the ID of >> * the bus master); some are specific to a particular kind of >> @@ -35,6 +42,19 @@ typedef struct MemTxAttrs { >> unsigned int secure:1; >> /* Memory access is usermode (unprivileged) */ >> unsigned int user:1; >> + /* >> + * Bus memory access permission. >> + * >> + * Some devices (such DMA) might be restricted to only access >> + * some type of device, such RAM devices. By default memory >> + * accesses are unspecified (MEMTXPERM_UNSPECIFIED), but could be >> + * unrestricted (MEMTXPERM_UNRESTRICTED, similar to an allow list) >> + * or restricted to a type of devices (similar to a deny list). >> + * Currently only RAM devices can be restricted (MEMTXPERM_RAM_DEVICE). > > I don't understand these 3 categories. > > MEMTXPERM_UNSPECIFIED means any MemoryRegion can be accessed?
MEMTXPERM_UNSPECIFIED means no change in the current behavior. IOW we haven't reviewed the device, and don't know whether it has to be restricted or not. > What does MEMTXPERM_UNRESTRICTED mean? How does this differ from > MEMTXPERM_UNSPECIFIED? We set MEMTXPERM_UNRESTRICTED when we reviewed a device and are sure it can be accessed any region (even being re-entrant on itself). I understand it like connected via a dual-port on the bus, and allowing read *and* write accesses at the same time... So the device is allowed to access itself, i.e. it can re-program itself in loop and so on. IIUC this is a requested feature for this API. > What exactly does MEMTXPERM_RAM_DEVICE mean? Maybe that only > MemoryRegions where memory_region_is_ram() is true can be accessed? No, it means, while we don't know which bus owner will access the device, the device itself can only access RAMd regions on the bus. I added MEMTXPERM_RAM_DEVICE as the first example, thinking it is the more generic use case. But maybe it is too generic and unuseful, and we need a real bus permission matrix?