Re: [Xen-devel] [PATCH v9 14/19] xen: introduce xenpv bus and a dummy pvcpu device

On 01/06/2014 09:46 AM, Roger Pau Monné wrote:
> On 05/01/14 22:52, Julien Grall wrote:
>>
>>
>> On 01/02/2014 03:43 PM, Roger Pau Monne wrote:
>>> Since Xen PVH guests doesn't have ACPI, we need to create a dummy
>>> bus so top level Xen devices can attach to it (instead of
>>> attaching directly to the nexus) and a pvcpu device that will be used
>>> to fill the pcpu->pc_device field.
>>> ---
>>>    sys/conf/files.amd64 |    1 +
>>>    sys/conf/files.i386  |    1 +
>>>    sys/x86/xen/xenpv.c  |  155
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>
>> I think it makes more sense to have 2 files: one for xenpv bus and one
>> for a dummy pvcpu device. It would allow us to move xenpv bus to common
>> code (sys/xen or sys/dev/xen).
>
> Ack. I wasn't thinking other arches will probably use the xenpv bus but
> not the dummy cpu device. Would you agree to leave xenpv bus inside
> x86/xen for now and move the dummy PV cpu device to dev/xen/pvcpu/?

As we will attach every xen device to xenpv, it makes more sense to have 
xenpv bus used on ARM. It will avoid duplication code and keep it nicer.

I'm fine with this solution for now. I will update/move the code when I 
will send the patch series to support FreeBSD on Xen on ARM.

>>
>> [..]
>>
>>> +
>>> +static int
>>> +xenpv_probe(device_t dev)
>>> +{
>>> +
>>> +    device_set_desc(dev, "Xen PV bus");
>>> +    device_quiet(dev);
>>> +    return (0);
>>
>> As I understand, 0 means I can "handle" the current device, in this case
>> if a device is probing, because it doesn't have yet a driver, we will
>> use xenpv and end up with 2 (or even more) xenpv buses.
>>
>> As we only want to probe xenpv bus once, when the bus was added
>> manually, returning BUS_PROBE_NO_WILDCARD would suit better.
>>
>> [..]
>>
>>> +static int
>>> +xenpvcpu_probe(device_t dev)
>>> +{
>>> +
>>> +    device_set_desc(dev, "Xen PV CPU");
>>> +    return (0);
>>
>> Same here: BUS_PROBE_NOWILDCARD.
>
> Ack for both, will change it to BUS_PROBE_NOWILDCARD. While at it, we
> should also change xenstore probe function to return BUS_PROBE_NOWILDCARD.
>

Right, I have a patch for xenstore. Do you want me to send it?

-- 
Julien Grall