On Tue, Aug 28, 2012 at 11:09 AM, liu ping fan <qemul...@gmail.com> wrote: > On Tue, Aug 28, 2012 at 3:38 AM, Jan Kiszka <jan.kis...@siemens.com> wrote: >> On 2012-08-27 20:52, Avi Kivity wrote: >>> On 08/27/2012 11:39 AM, Jan Kiszka wrote: >>>> On 2012-08-27 20:20, Avi Kivity wrote: >>>>> On 08/27/2012 11:17 AM, Jan Kiszka wrote: >>>>>> On 2012-08-27 20:09, Avi Kivity wrote: >>>>>>> On 08/27/2012 10:14 AM, Jan Kiszka wrote: >>>>>>>>> >>>>>>>>> Deregistration is fine, the problem is destruction. >>>>>>>>> >>>>>>>> >>>>>>>> It isn't as you access memory region states that can change after >>>>>>>> deregistration. Devices can remove memory regions from the mapping, >>>>>>>> alter and then reinsert them. The last to steps must not happen while >>>>>>>> anyone is still using a reference to that region. >>>>>>>> >>>>>>> >>>>>>> Why not? If the guest is accessing an mmio region while reconfiguring >>>>>>> it in a way that changes its meaning, either the previous or the next >>>>>>> meaning is valid. >>>>>> >>>>>> If the memory region owner sets the content to zero or even releases it >>>>>> (nothing states a memory region can only live inside a device >>>>>> structure), we will crash. Restricting how a memory region can be >>>>>> created and handled after it was once registered somewhere is an >>>>>> unnatural interface, waiting to cause subtle bugs. >>>>> >>>>> Using an Object * allows the simple case to be really simple (object == >>>>> device) and the hard cases to be doable. >>>>> >>>>> What would you suggest as a better interface? >>>> >>>> To protect the life cycle of the object we manage in the memory layer: >>>> regions. We don't manage devices there. If there is any implementation >>>> benefit in having a full QOM object, then make memory regions objects. >>> >>> I see and sympathise with this point of view. >>> >>> The idea to use opaque and convert it to Object * is that often a device >>> has multiple regions but no interest in managing them separately. So >>> managing regions will cause the device model authors to create >>> boilerplate code to push region management to device management. >>> >>> I had this experience with the first iterations of the region interface, >>> where instead of opaque we had MemoryRegion *. Device code would use >>> container_of() in the simple case, but the non-simple case caused lots >>> of pain. I believe it is the natural interface, but in practice it >>> turned out to cause too much churn. >>> >>>> I simply don't like this indirection, having the memory layer pick up >>>> the opaque value of the region and interpret it. >>> >>> That's just an intermediate step. The idea is that eventually opaque >>> will be changed to Object *. >>> >>>> Even worse, apply >>>> restrictions on how the dispatched objects, the regions, have to be >>>> treated because of this. >>> >>> Please elaborate. >> >> The fact that you can't manipulate a memory region object arbitrarily >> after removing it from the mapping because you track the references to >> the object that the region points to, not the region itself. The region >> remains in use by the dispatching layer and potentially the called >> device, even after deregistration. > > Why can it in use by dispatching layer? Memory region just servers as > interface for mem lookup, when dispatching in mr's MemoryRegionOps, > why do we need to access it? Which means that read/write can cause > register/deregister mr? Example?
I found example in pci_update_mappings(), but but my following point still stand. > Also I think MemoryRegionOps will eventually operate on Object (mr is > only interface). MRs is only represent of Object in the view of > memory(so their life cycle can be managed by Object). And there could > be some intermediate mr like those lie in subpage_t which are not > based on Object. But we can walk down to the terminal point and > extract the real Object, > so when dispatching, we only need to ensure that Object and its direct > mr are alive. > > Regards, > pingfan >> >> Jan >> >> -- >> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE >> Corporate Competence Center Embedded Linux
