[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 00/11] memory: Forbid mapping AddressSpace root MemoryRegi
From: |
Peter Xu |
Subject: |
Re: [PATCH v2 00/11] memory: Forbid mapping AddressSpace root MemoryRegion |
Date: |
Tue, 20 Apr 2021 15:07:09 -0400 |
On Sat, Apr 17, 2021 at 12:30:17PM +0200, Philippe Mathieu-Daudé wrote:
> AddressSpace are physical address view and shouldn't be using
> non-zero base address. The correct way to map a MR used as AS
> root is to use a MR alias.
Today when I rethink this, I figured another way (maybe easier?) to fix the
issue.
The major problem so far we had is that mr->addr can be anything for a root mr
if it's added as subregion of another mr.
E.g. in current implementation of mtree_print_mr() MR.addr is constantly used
as an offset value:
cur_start = base + mr->addr;
However afaict mr->addr is defined as "relative offset of this mr to its
container", as in memory_region_add_subregion_common(). Say, mr->addr is
undefined from that pov if mr->container==NULL, as this MR belongs to nobody.
And if it's defined, it's only meaning is in its container's context (or say,
address space) only.
That said, when we do mtree_print_mr(), another solution could be as simple as,
not referencing mr->addr if we _know_ we're working on the root mr, as this is
definitely _not_ in the context of the mr's container, even if it has one
container after all:
---8<---
diff --git a/softmmu/memory.c b/softmmu/memory.c
index d4493ef9e43..d71fb8ecc89 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2940,7 +2940,7 @@ static void mtree_print_mr(const MemoryRegion *mr,
unsigned int level,
return;
}
- cur_start = base + mr->addr;
+ cur_start = base + (level == 1) ? 0 : mr->addr;
cur_end = cur_start + MR_SIZE(mr->size);
/*
---8<---
Phil, do you think it'll work too to fix the strange offset value dumped in
"info mtree"?
I don't know (even if it works, perhaps I've missed something) which is better,
as current series seems cleaner, then any mr will either belong to a AS or a MR
(never both!), but just raise it up.
Thanks,
--
Peter Xu
- Re: [PATCH v2 05/11] hw/pci-host/raven: Add PCI_IO_BASE_ADDR definition, (continued)
- [PATCH v2 06/11] hw/pci-host/raven: Assert PCI I/O AddressSpace is based at 0x80000000, Philippe Mathieu-Daudé, 2021/04/17
- [PATCH v2 07/11] hw/pci-host/raven: Use MR alias for AS root, not sysbus mapped MR, Philippe Mathieu-Daudé, 2021/04/17
- [RFC PATCH v2 08/11] hw/pci-host/raven: Remove pointless alias mapping onto system bus, Philippe Mathieu-Daudé, 2021/04/17
- [PATCH v2 09/11] hw/pci-host/prep: Do not directly map bus-master region onto main bus, Philippe Mathieu-Daudé, 2021/04/17
- [PATCH v2 10/11] memory: Make sure root MR won't be added as subregion, Philippe Mathieu-Daudé, 2021/04/17
- [PATCH v2 11/11] hw/pci-host/raven: Remove temporary assertion 'root MR is zero-based', Philippe Mathieu-Daudé, 2021/04/17
- Re: [PATCH v2 00/11] memory: Forbid mapping AddressSpace root MemoryRegion, Cédric Le Goater, 2021/04/19
- Re: [PATCH v2 00/11] memory: Forbid mapping AddressSpace root MemoryRegion,
Peter Xu <=