| Message ID | 1409769375-22286-3-git-send-email-bogdan.purcareata@freescale.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2014-09-03 at 14:36 -0400, Bogdan Purcareata wrote: > This is done due to the fact that the kvm-openpic region_{add,del} callbacks > can be invoked for sections generated from other memory regions as well. These > callbacks should handle only requests for the kvm-openpic memory region. > > The patch fixes a bug on target-ppc occuring when the "e500-pci-bar0" memory > region is added. This memory region registers an alias to the "e500-ccsr" memory > region, which further contains the "kvm-openpic" subregion. Due to this alias, > the kvm_openpic_region_add is called once more, with an offset within the > "e500-pci-bar" memory region. This generates the remapping of the > in-kernel MPIC at a wrong offset. OK, so the problem is that we really do have the MPIC mapped in two locations (and the kernel API only lets us map one of them). It would be nice if the MemoryRegionSection struct indicated the alias being represented rather than needing to recalculate the non-aliased address. -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Friday, September 05, 2014 6:47 PM > To: Purcareata Bogdan-B43198 > Cc: qemu-ppc@nongnu.org; Caraman Mihai Claudiu-B02008; qemu-devel@nongnu.org > Subject: Re: [Qemu-ppc] [PATCH 2/2] PPC: openpic_kvm: Filter region callbacks > based on memory region offset > > On Wed, 2014-09-03 at 14:36 -0400, Bogdan Purcareata wrote: > > This is done due to the fact that the kvm-openpic region_{add,del} callbacks > > can be invoked for sections generated from other memory regions as well. > These > > callbacks should handle only requests for the kvm-openpic memory region. > > > > The patch fixes a bug on target-ppc occuring when the "e500-pci-bar0" memory > > region is added. This memory region registers an alias to the "e500-ccsr" > memory > > region, which further contains the "kvm-openpic" subregion. Due to this > alias, > > the kvm_openpic_region_add is called once more, with an offset within the > > "e500-pci-bar" memory region. This generates the remapping of the > > in-kernel MPIC at a wrong offset. > > OK, so the problem is that we really do have the MPIC mapped in two > locations (and the kernel API only lets us map one of them). It would > be nice if the MemoryRegionSection struct indicated the alias being > represented rather than needing to recalculate the non-aliased address. Not sure I fully understand the terminology of the alias being represented. In fact, out of the two mentioned in the previous mail, "e500-pci-bar0" and "e500-ccsr", I don't know which one is the "alias". If it's the former, thus "e500-pci-bar0" is an alias for "e500-ccsr", this information could be propagated down to the MemoryRegionSection. However, according to [1], "aliases may point to any type of region, including other aliases". So if the MemoryRegionSection has been built by going through a chain of aliases, all this information must be included in the structure. If it's the latter, thus "e500-ccsr" is an alias for "e500-pci-bar0", we can get to it in the current model as well. For our specific case, the MemoryRegionSection points to the "kvm-openpic" MemoryRegion, which in turn points to "e500-ccsr" as its parent (container). What do you think? [1] http://git.qemu.org/?p=qemu.git;a=blob;f=docs/memory.txt Thanks, Bogdan P.
diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c index e3bce04..45d8736 100644 --- a/hw/intc/openpic_kvm.c +++ b/hw/intc/openpic_kvm.c @@ -124,7 +124,9 @@ static void kvm_openpic_region_add(MemoryListener *listener, } /* Ignore events on regions that are not us */ - if (section->mr != &opp->mem) { + if (section->mr != &opp->mem || + section->offset_within_address_space != + memory_region_address_space_offset(section->mr)) { return; } @@ -151,7 +153,9 @@ static void kvm_openpic_region_del(MemoryListener *listener, int ret; /* Ignore events on regions that are not us */ - if (section->mr != &opp->mem) { + if (section->mr != &opp->mem || + section->offset_within_address_space != + memory_region_address_space_offset(section->mr)) { return; }
This is done due to the fact that the kvm-openpic region_{add,del} callbacks can be invoked for sections generated from other memory regions as well. These callbacks should handle only requests for the kvm-openpic memory region. The patch fixes a bug on target-ppc occuring when the "e500-pci-bar0" memory region is added. This memory region registers an alias to the "e500-ccsr" memory region, which further contains the "kvm-openpic" subregion. Due to this alias, the kvm_openpic_region_add is called once more, with an offset within the "e500-pci-bar" memory region. This generates the remapping of the in-kernel MPIC at a wrong offset. The fix consists in an additional filter in kvm_openpic_region_{add,del} to consider only addresses matching the start of the kvm-openpic memory region. Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com> --- hw/intc/openpic_kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)