[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v3 3/3] softmmu/physmem: Introduce MemTxAttrs::memory fie
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [RFC PATCH v3 3/3] softmmu/physmem: Introduce MemTxAttrs::memory field and MEMTX_BUS_ERROR |
Date: |
Sat, 18 Dec 2021 00:18:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 |
On 12/17/21 23:34, Peter Maydell wrote:
> On Wed, 15 Dec 2021 at 18:24, Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
>>
>> Add the 'memory' bit to the memory attributes to restrict bus
>> controller accesses to memories.
>>
>> Introduce flatview_access_allowed() to check bus permission
>> before running any bus transaction.
>>
>> Have read/write accessors return MEMTX_BUS_ERROR if an access is
>> restricted.
>>
>> There is no change for the default case where 'memory' is not set.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> include/exec/memattrs.h | 9 +++++++++
>> softmmu/physmem.c | 43 +++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
>> index 95f2d20d55b..f0063583ee2 100644
>> --- a/include/exec/memattrs.h
>> +++ b/include/exec/memattrs.h
>> @@ -35,6 +35,14 @@ typedef struct MemTxAttrs {
>> unsigned int secure:1;
>> /* Memory access is usermode (unprivileged) */
>> unsigned int user:1;
>> + /*
>> + * Bus interconnect and peripherals can access anything (memories,
>> + * devices) by default. By setting the 'memory' bit, bus transaction
>> + * are restricted to "normal" memories (per the AMBA documentation)
>> + * versus devices. Access to devices will be logged and rejected
>> + * (see MEMTX_BUS_ERROR).
>> + */
>> + unsigned int memory:1;
>> /* Requester ID (for MSI for example) */
>> unsigned int requester_id:16;
>> /* Invert endianness for this page */
>> @@ -66,6 +74,7 @@ typedef struct MemTxAttrs {
>> #define MEMTX_OK 0
>> #define MEMTX_ERROR (1U << 0) /* device returned an error */
>> #define MEMTX_DECODE_ERROR (1U << 1) /* nothing at that address */
>> +#define MEMTX_BUS_ERROR (1U << 2) /* bus returned an error */
>
> This is kind of odd naming, because MEMTX_DECODE_ERROR already means
> "bus/interconnect returned an error" and it generally translates
> into what at the OS level gets called a "bus error"...
MEMTX_DECODE_ERROR is "nothing at that address". We want a name
for "there is something, but you don't have access to it".
Maybe MEMTX_ILLEGAL_ERROR?