hw/s390x/s390-pci-inst.c | 2 +- include/system/memory.h | 2 +- system/memory.c | 25 ++++++++++++++++++++----- system/physmem.c | 3 ++- 4 files changed, 24 insertions(+), 8 deletions(-)
Hello,
I decided to propose this change after working in a RISC-V gitlab bug
[1]. In my proposed fix [2] I had to make assumptions about
address_space_ld* error return, which is always MEMTX_DECODE_ERROR, to
identify that in that particular scenario this is a permission error.
In RISC-V (and from what I can see other archs like x86) an ACCESS_ERROR
due to mem page permission issues will fire a different fault than other
errors like page not found.
The core logic is on memory_region_access_valid(), which already makes a
distinction between error reasons when we look at the qemu_log_mask()
errors, e.g. this is being characterized as a 'rejected':
if (mr->ops->valid.accepts
&& !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
", size %u, region '%s', reason: rejected\n",
is_write ? "write" : "read",
addr, size, memory_region_name(mr));
but memory_region_access_valid() just returns a bool, and we remain
oblivious about about the nature of the error found. Patch 1 adds an
extra parameter in memory_region_access_valid() to write the reason code
for a valid/invalid access.
Patch 2 changes memory_region_dispatch_(read|write) to return the reason
code from memory_region_access_valid() in case an error was found. This
will change error codes from DECODE_ERROR to ACCESS_ERROR when there's a
permission error involved, so before proposing this change I did a
search in all cases where DECODE_ERROR is handled via
memory_region_dispatch_* APIs. I didn't see any code where a
DECODE_ERROR is ignored (i.e treated as an OK result) and that would be
impacted by this change. Gitlab CI didn't complain about it either, so
I feel moderately confident that this change can be done without too
much harm.
[1] https://gitlab.com/qemu-project/qemu/-/work_items/3502
[2] https://lore.kernel.org/qemu-devel/20260522172502.320529-1-daniel.barboza@oss.qualcomm.com/
Daniel Henrique Barboza (2):
system/memory.c: add MemTxResult arg in memory_region_access_valid()
memory.c: read memory_region_access_valid() err reason in
memory_region_dispatch_*
hw/s390x/s390-pci-inst.c | 2 +-
include/system/memory.h | 2 +-
system/memory.c | 25 ++++++++++++++++++++-----
system/physmem.c | 3 ++-
4 files changed, 24 insertions(+), 8 deletions(-)
--
2.43.0
On Tue, 2 Jun 2026 at 20:09, Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> Hello,
>
> I decided to propose this change after working in a RISC-V gitlab bug
> [1]. In my proposed fix [2] I had to make assumptions about
> address_space_ld* error return, which is always MEMTX_DECODE_ERROR, to
> identify that in that particular scenario this is a permission error.
> In RISC-V (and from what I can see other archs like x86) an ACCESS_ERROR
> due to mem page permission issues will fire a different fault than other
> errors like page not found.
>
> The core logic is on memory_region_access_valid(), which already makes a
> distinction between error reasons when we look at the qemu_log_mask()
> errors, e.g. this is being characterized as a 'rejected':
>
> if (mr->ops->valid.accepts
> && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
> qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> ", size %u, region '%s', reason: rejected\n",
> is_write ? "write" : "read",
> addr, size, memory_region_name(mr));
valid.accepts returning false isn't a permissions error, though.
It's for cases like "you tried to do a byte access to this device
that requires 32-bit accesses only". Permissions errors are entirely
in the MMU, and don't appear in the MemTx APIs, which are about
transactions in the fabric outside the MMU. The CPU emulation
catches permissions errors early before it even does the
address_space_ld* call.
What is the equivalent of this in the underlying hardware?
The current error codes are intended to roughly match the AXI
bus SLVERR and DECERR error responses (which are "we were
able to pass this memory transaction to a device, but it
rejected it" and "we weren't able to find the device for
this memory transaction at all").
The valid.accepts callback failing is always "we know the memory
region but it doesn't like this", which is true for all
memory_region_access_valid() failures, so maybe the problem
here is that we return DECODE_ERROR for that case when
we should be returning ACCESS_ERROR ?
thanks
-- PMM
On 6/8/2026 6:09 AM, Peter Maydell wrote:
> On Tue, 2 Jun 2026 at 20:09, Daniel Henrique Barboza
> <daniel.barboza@oss.qualcomm.com> wrote:
>>
>> Hello,
>>
>> I decided to propose this change after working in a RISC-V gitlab bug
>> [1]. In my proposed fix [2] I had to make assumptions about
>> address_space_ld* error return, which is always MEMTX_DECODE_ERROR, to
>> identify that in that particular scenario this is a permission error.
>> In RISC-V (and from what I can see other archs like x86) an ACCESS_ERROR
>> due to mem page permission issues will fire a different fault than other
>> errors like page not found.
>>
>> The core logic is on memory_region_access_valid(), which already makes a
>> distinction between error reasons when we look at the qemu_log_mask()
>> errors, e.g. this is being characterized as a 'rejected':
>>
>> if (mr->ops->valid.accepts
>> && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
>> qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
>> ", size %u, region '%s', reason: rejected\n",
>> is_write ? "write" : "read",
>> addr, size, memory_region_name(mr));
>
> valid.accepts returning false isn't a permissions error, though.
> It's for cases like "you tried to do a byte access to this device
> that requires 32-bit accesses only". Permissions errors are entirely
> in the MMU, and don't appear in the MemTx APIs, which are about
> transactions in the fabric outside the MMU. The CPU emulation
> catches permissions errors early before it even does the
> address_space_ld* call.
>
> What is the equivalent of this in the underlying hardware?
> The current error codes are intended to roughly match the AXI
> bus SLVERR and DECERR error responses (which are "we were
> able to pass this memory transaction to a device, but it
> rejected it" and "we weren't able to find the device for
> this memory transaction at all").
>
> The valid.accepts callback failing is always "we know the memory
> region but it doesn't like this", which is true for all
> memory_region_access_valid() failures, so maybe the problem
> here is that we return DECODE_ERROR for that case when
> we should be returning ACCESS_ERROR ?
This is the end result I'm going for. In patch 1 I changed memory_region_access_valid()
to return ACCESS_ERROR when the valid.accepts fails, keeping all the
other exit code as DECODE_ERROR. The ACCESS_ERROR will then be propagated
as an error in target/riscv/cpu_helper.c, get_physical_address():
if (riscv_cpu_mxl(env) == MXL_RV32) {
pte = be ? address_space_ldl_be(cs->as, pte_addr, attrs, &res)
: address_space_ldl_le(cs->as, pte_addr, attrs, &res);
} else {
pte = be ? address_space_ldq_be(cs->as, pte_addr, attrs, &res)
: address_space_ldq_le(cs->as, pte_addr, attrs, &res);
}
if (res != MEMTX_OK) {
return TRANSLATE_FAIL;
}
In this particular case 'res' will fail as ACCESS_ERROR if the pte does not have
the right permissions and we can throw the right exception.
Now, considering what you said:
"Permissions errors are entirely in the MMU, and don't appear in the MemTx APIs,
which are about transactions in the fabric outside the MMU. The CPU emulation
catches permissions errors early before it even does the address_space_ld* call."
I can say that this isn't happening in the RISC-V backend at least, so maybe the
core issue here is that RISC-V is skipping steps in address translation/validation.
Thanks,
Daniel
>
> thanks
> -- PMM
On Mon, 8 Jun 2026 at 10:47, Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
>
>
> On 6/8/2026 6:09 AM, Peter Maydell wrote:
> > On Tue, 2 Jun 2026 at 20:09, Daniel Henrique Barboza
> > <daniel.barboza@oss.qualcomm.com> wrote:
> >>
> >> Hello,
> >>
> >> I decided to propose this change after working in a RISC-V gitlab bug
> >> [1]. In my proposed fix [2] I had to make assumptions about
> >> address_space_ld* error return, which is always MEMTX_DECODE_ERROR, to
> >> identify that in that particular scenario this is a permission error.
> >> In RISC-V (and from what I can see other archs like x86) an ACCESS_ERROR
> >> due to mem page permission issues will fire a different fault than other
> >> errors like page not found.
> >>
> >> The core logic is on memory_region_access_valid(), which already makes a
> >> distinction between error reasons when we look at the qemu_log_mask()
> >> errors, e.g. this is being characterized as a 'rejected':
> >>
> >> if (mr->ops->valid.accepts
> >> && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
> >> qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> >> ", size %u, region '%s', reason: rejected\n",
> >> is_write ? "write" : "read",
> >> addr, size, memory_region_name(mr));
> >
> > valid.accepts returning false isn't a permissions error, though.
> > It's for cases like "you tried to do a byte access to this device
> > that requires 32-bit accesses only". Permissions errors are entirely
> > in the MMU, and don't appear in the MemTx APIs, which are about
> > transactions in the fabric outside the MMU. The CPU emulation
> > catches permissions errors early before it even does the
> > address_space_ld* call.
> >
> > What is the equivalent of this in the underlying hardware?
> > The current error codes are intended to roughly match the AXI
> > bus SLVERR and DECERR error responses (which are "we were
> > able to pass this memory transaction to a device, but it
> > rejected it" and "we weren't able to find the device for
> > this memory transaction at all").
> >
> > The valid.accepts callback failing is always "we know the memory
> > region but it doesn't like this", which is true for all
> > memory_region_access_valid() failures, so maybe the problem
> > here is that we return DECODE_ERROR for that case when
> > we should be returning ACCESS_ERROR ?
>
> This is the end result I'm going for. In patch 1 I changed memory_region_access_valid()
> to return ACCESS_ERROR when the valid.accepts fails, keeping all the
> other exit code as DECODE_ERROR. The ACCESS_ERROR will then be propagated
> as an error in target/riscv/cpu_helper.c, get_physical_address():
>
> if (riscv_cpu_mxl(env) == MXL_RV32) {
> pte = be ? address_space_ldl_be(cs->as, pte_addr, attrs, &res)
> : address_space_ldl_le(cs->as, pte_addr, attrs, &res);
> } else {
> pte = be ? address_space_ldq_be(cs->as, pte_addr, attrs, &res)
> : address_space_ldq_le(cs->as, pte_addr, attrs, &res);
> }
>
> if (res != MEMTX_OK) {
> return TRANSLATE_FAIL;
> }
>
> In this particular case 'res' will fail as ACCESS_ERROR if the pte does not have
> the right permissions and we can throw the right exception.
But what's the "right exception" here? This is, I think, the code
for "the MMU is loading the page table entry from external memory".
I'm not clear what this has to do with permissions. Generally
"permissions" is a concept that applies to virtual addresses, i.e.
"does the page table for this vaddr say we can access it?", and there's
no concept of permissions for physical addresses. The exception to
that is that you might have something like Arm's "granule protection
table" which does apply permissions to physical accesses, so an
in-guest-memory data structure defines what physical addresses can be
accessed in particular ways. In QEMU we model this as effectively
another level of page tables, so again the check happens before we
do the address_space_ld* call. For risc-v a quick look at the architecture
spec suggests that the Smepmp extension is a bit like this (but
handled via registers rather than guest-memory data structures).
The other kind of check that might need to be done in the MMU before
the load call is "is this physaddr out of range?" (e.g. CPU has
48 bit physaddrs and the L1 PTE specifies a physaddr for the L2 PTE
that is outside the 48 bit range).
Do you have a reference in the architecture spec for the check that
you're trying to implement here ?
I would be a bit surprised if the riscv architecture mandated different
MMU errors for "page table entry load failed because the physaddr is not
pointing at any device" vs "page table entry load failed because the
physaddr points at a device and the device rejected it", because it's
really up to the bus specification how clearly it distinguishes these
various kinds of external abort (or even whether it distinguishes them
synchronously vs asynchronously, given the existence of caches), and
usually architecture specifications try not to impose too much on
the implementation.
thanks
-- PMM
On 6/8/2026 7:08 AM, Peter Maydell wrote:
> On Mon, 8 Jun 2026 at 10:47, Daniel Henrique Barboza
> <daniel.barboza@oss.qualcomm.com> wrote:
>>
>>
>>
>> On 6/8/2026 6:09 AM, Peter Maydell wrote:
>>> On Tue, 2 Jun 2026 at 20:09, Daniel Henrique Barboza
>>> <daniel.barboza@oss.qualcomm.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> I decided to propose this change after working in a RISC-V gitlab bug
>>>> [1]. In my proposed fix [2] I had to make assumptions about
>>>> address_space_ld* error return, which is always MEMTX_DECODE_ERROR, to
>>>> identify that in that particular scenario this is a permission error.
>>>> In RISC-V (and from what I can see other archs like x86) an ACCESS_ERROR
>>>> due to mem page permission issues will fire a different fault than other
>>>> errors like page not found.
>>>>
>>>> The core logic is on memory_region_access_valid(), which already makes a
>>>> distinction between error reasons when we look at the qemu_log_mask()
>>>> errors, e.g. this is being characterized as a 'rejected':
>>>>
>>>> if (mr->ops->valid.accepts
>>>> && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
>>>> qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
>>>> ", size %u, region '%s', reason: rejected\n",
>>>> is_write ? "write" : "read",
>>>> addr, size, memory_region_name(mr));
>>>
>>> valid.accepts returning false isn't a permissions error, though.
>>> It's for cases like "you tried to do a byte access to this device
>>> that requires 32-bit accesses only". Permissions errors are entirely
>>> in the MMU, and don't appear in the MemTx APIs, which are about
>>> transactions in the fabric outside the MMU. The CPU emulation
>>> catches permissions errors early before it even does the
>>> address_space_ld* call.
>>>
>>> What is the equivalent of this in the underlying hardware?
>>> The current error codes are intended to roughly match the AXI
>>> bus SLVERR and DECERR error responses (which are "we were
>>> able to pass this memory transaction to a device, but it
>>> rejected it" and "we weren't able to find the device for
>>> this memory transaction at all").
>>>
>>> The valid.accepts callback failing is always "we know the memory
>>> region but it doesn't like this", which is true for all
>>> memory_region_access_valid() failures, so maybe the problem
>>> here is that we return DECODE_ERROR for that case when
>>> we should be returning ACCESS_ERROR ?
>>
>> This is the end result I'm going for. In patch 1 I changed memory_region_access_valid()
>> to return ACCESS_ERROR when the valid.accepts fails, keeping all the
>> other exit code as DECODE_ERROR. The ACCESS_ERROR will then be propagated
>> as an error in target/riscv/cpu_helper.c, get_physical_address():
>>
>> if (riscv_cpu_mxl(env) == MXL_RV32) {
>> pte = be ? address_space_ldl_be(cs->as, pte_addr, attrs, &res)
>> : address_space_ldl_le(cs->as, pte_addr, attrs, &res);
>> } else {
>> pte = be ? address_space_ldq_be(cs->as, pte_addr, attrs, &res)
>> : address_space_ldq_le(cs->as, pte_addr, attrs, &res);
>> }
>>
>> if (res != MEMTX_OK) {
>> return TRANSLATE_FAIL;
>> }
>>
>> In this particular case 'res' will fail as ACCESS_ERROR if the pte does not have
>> the right permissions and we can throw the right exception.
>
> But what's the "right exception" here? This is, I think, the code
> for "the MMU is loading the page table entry from external memory".
> I'm not clear what this has to do with permissions. Generally
> "permissions" is a concept that applies to virtual addresses, i.e.
> "does the page table for this vaddr say we can access it?", and there's
> no concept of permissions for physical addresses. The exception to
> that is that you might have something like Arm's "granule protection
> table" which does apply permissions to physical accesses, so an
> in-guest-memory data structure defines what physical addresses can be
> accessed in particular ways. In QEMU we model this as effectively
> another level of page tables, so again the check happens before we
> do the address_space_ld* call. For risc-v a quick look at the architecture
> spec suggests that the Smepmp extension is a bit like this (but
> handled via registers rather than guest-memory data structures).
>
> The other kind of check that might need to be done in the MMU before
> the load call is "is this physaddr out of range?" (e.g. CPU has
> 48 bit physaddrs and the L1 PTE specifies a physaddr for the L2 PTE
> that is outside the 48 bit range).
>
> Do you have a reference in the architecture spec for the check that
> you're trying to implement here ?
Sure. It's on the RISC-V Privileged ISA, chapter 12, section
12.3.2 "Virtual Address Translation Process":
https://docs.riscv.org/reference/isa/_attachments/riscv-privileged.pdf
9. If pte.a=0, or if the original memory access is a store and pte.d=0:
- If the Svade extension is implemented, stop and raise a page-fault
exception corresponding to the original access type
- If a store to the PTE at address a+va.vpn[i]×PTESIZE would violate a
PMA or PMP check, raise an access-fault exception corresponding to
the original access type.
At this moment we're throwing a page-fault exception for both cases. We
want the second case to throw an access-fault exception. PMA stands for
"Physical Memory Access", i.e. the read/write/execute bits of the PTE.
PMP is an optional physical-memory protection scheme. Both are related
to a PTE permission to be read/write/executed, regardless of other
failures like address misaligned/wrong operation size.
Thanks,
Daniel
>
> I would be a bit surprised if the riscv architecture mandated different
> MMU errors for "page table entry load failed because the physaddr is not
> pointing at any device" vs "page table entry load failed because the
> physaddr points at a device and the device rejected it", because it's
> really up to the bus specification how clearly it distinguishes these
> various kinds of external abort (or even whether it distinguishes them
> synchronously vs asynchronously, given the existence of caches), and
> usually architecture specifications try not to impose too much on
> the implementation.
>
> thanks
> -- PMM
On Mon, 8 Jun 2026 at 13:16, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> wrote: > On 6/8/2026 7:08 AM, Peter Maydell wrote: > > On Mon, 8 Jun 2026 at 10:47, Daniel Henrique Barboza > > <daniel.barboza@oss.qualcomm.com> wrote: > > Do you have a reference in the architecture spec for the check that > > you're trying to implement here ? > > Sure. It's on the RISC-V Privileged ISA, chapter 12, section > 12.3.2 "Virtual Address Translation Process": > > https://docs.riscv.org/reference/isa/_attachments/riscv-privileged.pdf > > 9. If pte.a=0, or if the original memory access is a store and pte.d=0: > - If the Svade extension is implemented, stop and raise a page-fault > exception corresponding to the original access type > - If a store to the PTE at address a+va.vpn[i]×PTESIZE would violate a > PMA or PMP check, raise an access-fault exception corresponding to > the original access type. > > At this moment we're throwing a page-fault exception for both cases. We > want the second case to throw an access-fault exception. PMA stands for > "Physical Memory Access", i.e. the read/write/execute bits of the PTE. > PMP is an optional physical-memory protection scheme. Both are related > to a PTE permission to be read/write/executed, regardless of other > failures like address misaligned/wrong operation size. Are you sure that's what a PMA check is ? Section 3.6 seems to me to be describing something else, where the hardware uses some combination of hardwired info and unspecified platform-specific registers to determine whether the physical address is to somewhere that supports particular access widths, atomic insns, cacheability, and ability to read, write or exec. I can't see where in the spec it defines what's supposed to happen if the page-table-entry read fails because it's to an invalid physical address, so presumably that's just a subtype of "failed a PMA check". (3.6.1 has a note that supports that reading, where it says that previous spec versions had a "vacant region" concept, presumably corresponding to our "decode error", but that these are now treated as I/O regions that have none of r/w/x access permissions.) In that case the riscv page table walk code should: - fail with page-fault exception for "pte.a field is zero" (and do not attempt a load from physaddr 0) - otherwise, do the load, and report any kind of failure of the load as an access-fault exception Do you implement emulation of these platform-specific PMA registers? If so, those sound to me like they should be some extra level of checks that your page-table-walk code does before it does the real physical-address load, similar to how you do the PMP checks. It doesn't sound to me quite the same as the return value from QEMU's address_space_ld* functions, which is more the case of "the guest misconfigured the PMA registers and did an access that passed the PMA checks but which the actual device rejected". thanks -- PMM
Hi Peter, On 6/8/2026 9:52 AM, Peter Maydell wrote: > On Mon, 8 Jun 2026 at 13:16, Daniel Henrique Barboza > <daniel.barboza@oss.qualcomm.com> wrote: >> On 6/8/2026 7:08 AM, Peter Maydell wrote: >>> On Mon, 8 Jun 2026 at 10:47, Daniel Henrique Barboza >>> <daniel.barboza@oss.qualcomm.com> wrote: >>> Do you have a reference in the architecture spec for the check that >>> you're trying to implement here ? >> >> Sure. It's on the RISC-V Privileged ISA, chapter 12, section >> 12.3.2 "Virtual Address Translation Process": >> >> https://docs.riscv.org/reference/isa/_attachments/riscv-privileged.pdf >> >> 9. If pte.a=0, or if the original memory access is a store and pte.d=0: >> - If the Svade extension is implemented, stop and raise a page-fault >> exception corresponding to the original access type >> - If a store to the PTE at address a+va.vpn[i]×PTESIZE would violate a >> PMA or PMP check, raise an access-fault exception corresponding to >> the original access type. >> >> At this moment we're throwing a page-fault exception for both cases. We >> want the second case to throw an access-fault exception. PMA stands for >> "Physical Memory Access", i.e. the read/write/execute bits of the PTE. >> PMP is an optional physical-memory protection scheme. Both are related >> to a PTE permission to be read/write/executed, regardless of other >> failures like address misaligned/wrong operation size. > > Are you sure that's what a PMA check is ? Section 3.6 seems to me > to be describing something else, where the hardware uses some > combination of hardwired info and unspecified platform-specific > registers to determine whether the physical address is to somewhere > that supports particular access widths, atomic insns, cacheability, > and ability to read, write or exec. > > I can't see where in the spec it defines what's supposed to happen > if the page-table-entry read fails because it's to an invalid > physical address, so presumably that's just a subtype of "failed > a PMA check". (3.6.1 has a note that supports that reading, where > it says that previous spec versions had a "vacant region" concept, > presumably corresponding to our "decode error", but that these are > now treated as I/O regions that have none of r/w/x access permissions.) Sorry taking this long to answer. Yeah, "PMA check" as far as the RISC-V seems to understand is a combination of a lot of things that includes not just read/write/exec permissions but also alignment, atomicity, caching and so on. All memory accesses failures we're able to detect in the code, aside from the usual "address not found" error that we can interpret safely as an page fault (... I think ...), can be considered a PMA error. Which is something that we're *not* doing at this moment, and this might be an alternative for what I'm proposing here. It might come to what you said below: an extra layer of checks that we would call "PMA". I'll have to think more about it ... Cheers, Daniel > > In that case the riscv page table walk code should: > - fail with page-fault exception for "pte.a field is zero" > (and do not attempt a load from physaddr 0) > - otherwise, do the load, and report any kind of failure of the > load as an access-fault exception > > Do you implement emulation of these platform-specific PMA registers? > If so, those sound to me like they should be some extra level of > checks that your page-table-walk code does before it does the > real physical-address load, similar to how you do the PMP checks. > It doesn't sound to me quite the same as the return value from > QEMU's address_space_ld* functions, which is more the case of > "the guest misconfigured the PMA registers and did an access > that passed the PMA checks but which the actual device rejected". > > thanks > -- PMM
© 2016 - 2026 Red Hat, Inc.