Re: [Qemu-ppc] [RFC ppc-next PATCH 3/6] memory: add memory_region_to_address()

[Scott Wood]

On 03/21/2013 06:51:57 AM, Alexander Graf wrote:
> On 21.03.2013, at 12:49, Alexander Graf wrote:
>
> >
> > On 21.03.2013, at 12:44, Peter Maydell wrote:
> >
> >> On 21 March 2013 11:38, Alexander Graf <address@hidden> wrote:
> >>>
> >>> On 21.03.2013, at 12:32, Peter Maydell wrote:
> >>>
> >>>> On 21 March 2013 11:29, Alexander Graf <address@hidden> wrote:
> >>>>> On 21.03.2013, at 12:22, Peter Maydell wrote:
> >>>>>> We already nest the VGIC inside another memory region (the  
> a15mpcore
> >>>>>> container), and it works fine. This function is just iterating  
> through
> >>>>>> "everything any device asked me to tell the kernel about".
> >>>>>
> >>>>> So kda is the real physical offset? I'm having a hard time  
> reading that code :). According to this function:
> >>>>>
> >>>>> static void kvm_arm_devlistener_add(MemoryListener *listener,
> >>>>>                                  MemoryRegionSection *section)
> >>>>> {
> >>>>>  KVMDevice *kd;
> >>>>>
> >>>>>  QSLIST_FOREACH(kd, &kvm_devices_head, entries) {
> >>>>>      if (section->mr == kd->mr) {
> >>>>>          kd->kda.addr = section->offset_within_address_space;
> >>>>>      }
> >>>>>  }
> >>>>> }

What if the update is to a parent memory region, not to the one  
directly associated with the device?

Or does add() get called for all child regions (recursively) in such  
cases?

> >>> The distinction on whether a region is handled by KVM really needs
> >>> to be done by the device model.
> >>
> >> It is -- the device model is what calls kvm_arm_register_device().
> >> It's just the mechanics of "how do we tell the kernel the right
> >> address for this region at the point when we know it" that are
> >> handled in kvm.c.
> >
> > I think I'm slowly grasping what you're aiming at :). Ok, that  
> works. You do actually do the listener in the device model, just that  
> you pass code responsibility over to kvm.c.
> >
> > That's perfectly valid and sounds like a good model that Scott  
> probably wants to follow as well :).
>
> s/follow/evaluate/ :).
>
> The currently proposed device api doesn't have a generic notion of  
> device regions. Regions are a per-device property, because a single  
> device can have multiple regions.
>
> However, maybe with a bit of brainstorming we could come up with a  
> reasonably generic scheme.

In the kernel API?  Or do you mean a generic scheme within QEMU that  
encodes any reasonably expected mechanism for setting the device adress  
(e.g. assume that it is either a 64-bit attribute, or uses the legacy  
ARM API), or perhaps a callback into device code?

The MPIC's memory listener isn't that much code... I'm not sure there's  
a great need for a central KVM registry.

-Scott