qemu-devel
[Top][All Lists]
Advanced

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

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


From: Alexander Graf
Subject: Re: [Qemu-devel] [RFC ppc-next PATCH 3/6] memory: add memory_region_to_address()
Date: Thu, 21 Mar 2013 12:14:14 +0100

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

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?


Alex

> 
>> This patch adds a function that allows kvm-openpic to fetch its
>> base flat address from the MemoryListener.
> 
> This sounds to me like the wrong way to do it -- it's board
> models that decide where devices are mapped and the device
> code itself shouldn't have to know where it has been mapped.
> 
> -- PMM




reply via email to

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