[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [RFC ppc-next PATCH 3/6] memory: add memory_region_to_add

From: Peter Maydell
Subject: Re: [Qemu-ppc] [RFC ppc-next PATCH 3/6] memory: add memory_region_to_address()
Date: Thu, 21 Mar 2013 11:22:26 +0000

On 21 March 2013 11:14, Alexander Graf <address@hidden> wrote:
> On 21.03.2013, at 12:09, Peter Maydell wrote:
>> On 21 March 2013 11:05, Alexander Graf <address@hidden> wrote:
>>> On 21.03.2013, at 12:01, Peter Maydell wrote:
>>>> On 21 March 2013 10:59, Alexander Graf <address@hidden> wrote:
>>>>> On 21.03.2013, at 11:53, Peter Maydell wrote:
>>>>>> Check kvm_arm_register_device() in target-arm/kvm.c. Basically
>>>>>> the VGIC device model calls this function to say "tell the kernel
>>>>>> where this MemoryRegion is in the system address space, when it
>>>>>> eventually gets mapped". The code in kvm.c uses the memory system's
>>>>>> Notifier API to get a callback when the region is mapped into
>>>>>> an address space, which it uses to track the offset in the
>>>>>> address space. Finally, we use a machine init notifier so that
>>>>>> just before everything finally starts we can make the KVM ioctls
>>>>>> to say "here is where everything lives".
>>>>> Same thing here. The question is how the kvm-vgic code in QEMU
>>>>> finds out where it got mapped to. Scott adds this patch to do
>>>>> this, but I'd assume you have some other way :)
>>>> Hmm? The kvm-vgic code in QEMU doesn't need to know where it
>>>> lives. We have to tell the kernel so it can map its bits of
>>>> registers in at the right place, that's all.
>>> The kvm-vgic code in QEMU needs to tell the kernel, no? For
>>> that, it needs to know what to tell the kernel.
>> No. As I explained earlier, all the kvm-vgic code needs to do
>> is call kvm_arm_register_device(). That code in kvm.c then takes
>> care of telling the kernel. hw/kvm/arm_gic.c itself never knows or
>> needs to know where it's mapped. This is the whole point of the
>> mechanism involving notifiers.
> I fully disagree. Code that talks to the in-kernel device should
> live in hw/kvm/<device>.c, not in some random target-XXX/kvm.c file.

The code in kvm.c is entirely generic -- it provides a mechanism
for a device to say "this memory region is kernel ID X and it will
want to know where it lives". The kvm.c code will work for any device
with a memory mapped region, whether it's the GIC or something else.

> Of course the board defines where the device gets mapped to, but
> the communication with the in-kernel device bits should really be
> contained to the device itself.

You're arguing that every device should implement its own set
of notifier functions so it can get called back when its memory
regions are finally mapped, just so it can make a non-device-specific
KVM ioctl? The obvious thing to do is abstract that functionality

> So this is the function that gets invoked on ARM:
> static void kvm_arm_machine_init_done(Notifier *notifier, void *data)
> {
>     KVMDevice *kd, *tkd;
>     memory_listener_unregister(&devlistener);
>     QSLIST_FOREACH_SAFE(kd, &kvm_devices_head, entries, tkd) {
>         if (kd->kda.addr != -1) {
>             if (kvm_vm_ioctl(kvm_state, KVM_ARM_SET_DEVICE_ADDR,
>                              &kd->kda) < 0) {
>                 fprintf(stderr, "KVM_ARM_SET_DEVICE_ADDRESS failed: %s\n",
>                         strerror(errno));
>                 abort();
>             }
>         }
>         g_free(kd);
>     }
> }
> This only goes one level deep, right? So if you ever have to nest the
> VGIC inside of another MemoryRegion, this will break, right?

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".

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]