All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: liu ping fan <qemulist@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [Qemu-devel] memory: could we add extra input param for memory_region_init_io()?
Date: Tue, 21 Aug 2012 15:41:18 +0300	[thread overview]
Message-ID: <503381EE.9060003@redhat.com> (raw)
In-Reply-To: <CAJnKYQ=WSc4jZsJz0NB8zReWPxS4WPc0X6a9hUYCAM36j8eqgg@mail.gmail.com>

On 08/21/2012 02:18 PM, liu ping fan wrote:
>>
>>> But as
>>> it will also take the code path which has object_ref(Object*), so it
>>> has to convert, otherwise the code will corrupt.
>>> That is what I want to express.
>>
>> Option 2, for example, had MemoryRegionOps::object(MemoryRegion *) which
>> can be used to convert the opaque to an Object, or return NULL.  With
>> that option, there would be no corruption:
>>
> If we did not pass extra flag to indicate whether opaque is Object or
> not, we can not implement MemoryRegionOps::object().  Because we lack
> of something like "try,catch", and object_dynamic_cast() can not work.
> So besides memory_region_init_io(), we need extra interface to pass that flag?

The interface is MemoryRegionOps::object().

If the user does not implement it, then the opaque cannot be converted
into an object.  If the user did implement it, then the function
specifies how to convert it.

For example:


static Object *e1000_mmio_object(void *opaque)
{
    E1000State *s = opaque;

    return &s->dev.qdev.parent_obj;
}

static const MemoryRegionOps e1000_mmio_ops = {
    .read = e1000_mmio_read,
    .write = e1000_mmio_write,
    .endianness = DEVICE_LITTLE_ENDIAN,
    .object = e1000_mmio_object,
    .impl = {
        .min_access_size = 4,
        .max_access_size = 4,
    },
};

>>   qemu_mutex_lock(&mem_map_lock);
>>   MemoryRegionSection mrs = walk(&phys_map);
>>   Object *object = mrs.mr->ops->object(mrs.mr);
>>   if (object) {
>>       object_ref(object);
>>   }
>>   qemu_mutex_unlock(&mem_map_unlock);
>>

This should read:


    Object *object = NULL;
    if (mrs.mr->ops->object) {
        object = mrs.mr->ops->object(mrs.mr);
    }


>> So there is no memory corruption.  However, as I point out below, we
>> also lack the reference count.  Maybe we could do something like:
>>
> I think the hotplug issue is only for limited type of bus. And
> fortunately, for pci, they have already adopt Object.
> So for other SoC, maybe we can ignore this issue at current step

We still have to take the big qemu lock somehow.

>> If we can avoid a cycle.  Won't the device need to hold refs to the BAR?
> 
> I will check the code, but I has thought BAR is implemented by
> assigned device?  

Yes.


-- 
error compiling committee.c: too many arguments to function

  reply	other threads:[~2012-08-21 12:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-14  8:30 [Qemu-devel] memory: could we add extra input param for memory_region_init_io()? liu ping fan
2012-08-14 10:49 ` Avi Kivity
2012-08-16  3:22   ` liu ping fan
2012-08-16  9:23     ` Avi Kivity
2012-08-17  2:52       ` liu ping fan
2012-08-17  7:41         ` liu ping fan
2012-08-19 10:12           ` Avi Kivity
2012-08-19 11:23             ` Peter Maydell
2012-08-19 11:36               ` Avi Kivity
2012-08-21  4:48                 ` liu ping fan
2012-08-21  8:57                   ` Avi Kivity
2012-08-21  9:25                     ` liu ping fan
2012-08-21 10:13                       ` Avi Kivity
2012-08-21 11:18                         ` liu ping fan
2012-08-21 12:41                           ` Avi Kivity [this message]
2012-08-24  9:47                             ` liu ping fan
2012-08-19 12:03               ` Peter Maydell
2012-08-19 13:05                 ` Blue Swirl
2012-08-19 10:10         ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503381EE.9060003@redhat.com \
    --to=avi@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.