On 1/2/21 12:19 AM, Peter Maydell wrote:
> On Fri, 1 Jan 2021 at 23:12, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>>
>> Per the datasheet (Chapter 5.7.1. "PCI address regions"),
>> the PCIMAP register:
>>
>> Map the 64Mbyte regions marked "PCI_Lo" in the CPU's memory map,
>> each of which can be assigned to any 64 Mbyte-aligned region of
>> PCI memory. The address appearing on the PCI bus consists of the
>> low 26 bits of the CPU physical address, with the high 6 bits
>> coming from the appropriate base6 field. Each of the three regions
>> is an independent window onto PCI memory, and can be positioned on
>> any 64Mbyte boundary in PCI space.
>>
>> Remap the 3 regions on reset and when PCIMAP is updated.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>> ---
>> hw/pci-host/bonito.c | 49 ++++++++++++++++++++++++++++++++------------
>> 1 file changed, 36 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>> index a99eced0657..c58eeaf504c 100644
>> --- a/hw/pci-host/bonito.c
>> +++ b/hw/pci-host/bonito.c
>> @@ -137,6 +137,10 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1)
>>
>> /* 4. PCI address map control */
>> #define BONITO_PCIMAP (0x10 >> 2) /* 0x110 */
>> +FIELD(PCIMAP, LO0, 0, 6)
>> +FIELD(PCIMAP, LO1, 6, 6)
>> +FIELD(PCIMAP, LO2, 12, 6)
>> +FIELD(PCIMAP, 2G, 18, 1)
>> #define BONITO_PCIMEMBASECFG (0x14 >> 2) /* 0x114 */
>> #define BONITO_PCIMAP_CFG (0x18 >> 2) /* 0x118 */
>>
>> @@ -237,6 +241,7 @@ struct BonitoState {
>> qemu_irq *pic;
>> PCIBonitoState *pci_dev;
>> MemoryRegion pci_mem;
>> + MemoryRegion pcimem_lo_alias[3];
>> };
>>
>> #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
>> @@ -245,6 +250,31 @@ OBJECT_DECLARE_SIMPLE_TYPE(BonitoState,
>> BONITO_PCI_HOST_BRIDGE)
>> #define TYPE_PCI_BONITO "Bonito"
>> OBJECT_DECLARE_SIMPLE_TYPE(PCIBonitoState, PCI_BONITO)
>>
>> +static void bonito_remap(PCIBonitoState *s)
>> +{
>> + static const char *const region_name[3] = {
>> + "pci.lomem0", "pci.lomem1", "pci.lomem2"
>> + };
>> + BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
>> +
>> + for (size_t i = 0; i < 3; i++) {
>> + uint32_t offset = extract32(s->regs[BONITO_PCIMAP], 6 * i, 6) << 26;
>> +
>> + if (memory_region_is_mapped(&bs->pcimem_lo_alias[i])) {
>> + memory_region_del_subregion(get_system_memory(),
>> + &bs->pcimem_lo_alias[i]);
>> + object_unparent(OBJECT(&bs->pcimem_lo_alias[i]));
>> + }
>> +
>> + memory_region_init_alias(&bs->pcimem_lo_alias[i], OBJECT(s),
>> + region_name[i], &bs->pci_mem,
>> + offset, 64 * MiB);
>> + memory_region_add_subregion(get_system_memory(),
>> + BONITO_PCILO_BASE + i * 64 * MiB,
>> + &bs->pcimem_lo_alias[i]);
>> + }
>
> Rather than delete-and-reinit-and-add, it's probably better to
> just create the subregions once at device startup, and then use
> memory_region_set_enabled() and memory_region_set_address()
> to manipulate whether the subregion is visible and what address
> in the system memory it is mapped at.
Great! Thanks Peter :) TIL these functions.
>From what I understand from memory_region_readd_subregion (called
from memory_region_set_address) using memory_region_set_enabled()
directly is enough.
