Re: (v2) Design proposal for RMRR fix

[George Dunlap <george.dunlap@eu.citrix.com>]

On 01/14/2015 02:47 PM, Jan Beulich wrote:
>>>> On 14.01.15 at 15:37, <george.dunlap@eu.citrix.com> wrote:
>> On 01/14/2015 02:32 PM, Jan Beulich wrote:
>>>>>> On 14.01.15 at 13:01, <george.dunlap@eu.citrix.com> wrote:
>>>> On 01/14/2015 10:24 AM, Jan Beulich wrote:
>>>>>>>> On 14.01.15 at 10:43, <kevin.tian@intel.com> wrote:
>>>>>>>  From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>>>> Sent: Wednesday, January 14, 2015 5:00 PM
>>>>>>>
>>>>>>>>>> On 14.01.15 at 09:06, <kevin.tian@intel.com> wrote:
>>>>>>>> Now the open is whether we want to fail domain creation for all of above
>>>>>>>> conflicts. user may choose to bear with conflicts at his own disposal, or
>>>>>>>> libxl doesn't want to fail conflicts as preparation for future
>>>>>>>> hotplug/migration.
>>>>>>>> One possible option is to add a per-region flag to specify whether treating
>>>>>>>> relevant conflict as an error, when libxl composes the list to domain
>>>>>>>> builder.
>>>>>>>> and this information will be saved in a user space database accessible to
>>>>>>>> all components and also waterfall to Xen hypervisor when libxl requests
>>>>>>>> actual device assignment.
>>>>>>>
>>>>>>> That's certainly a possibility, albeit saying (in the guest config) that
>>>>>>> a region to be reserved only when possible is about the same as
>>>>>>> not stating that region. If at all, I'd see the rmrr-host value be a
>>>>>>> tristate (don't, try, and force) to that effect.
>>>>>>>
>>>>>>
>>>>>> how about something like below with bi-state?
>>>>>>
>>>>>> for statically assigned device:
>>>>>> 	pci = [ "00:02.0, 0/1" ]
>>>>>> where '0/1' represents try/force (or use 'try/force', or have a meaningful 
>>>>>> attribute like rmrr_check=try/force?)
>>>>>
>>>>> As said many times before, for statically assigned devices such a flag
>>>>> makes no sense.
>>>>>
>>>>>> for other usages like hotplug/migration:
>>>>>> 	reserved_regions = [ 'host, 0/1', 'start, end, 0/1', 'start, end, 0/1', ...]
>>>>>> If 'host' is specified, it implies rmrr_host, besides user can specific 
>>>>>> explicit ranges according to his detail requirement.
>>>>>
>>>>> For host the flag makes sense, but for the explicitly specified regions
>>>>> - as said before - I don't think it does.
>>>>
>>>> You don't think there are any circumstances where an admin should be
>>>> allowed to "shoot himself in the foot" by assigning a device which he
>>>> knows the RMRRs conflict -- perhaps because he "knows" that the RMRRs
>>>> won't actually be used?
>>>
>>> I did advocate for allowing this, and continue to do so. But I think
>>> the necessary override for this would apply at assignment time,
>>> not when punching the holes (i.e. would need to be a different
>>> setting).
>>
>> But essentially what you're saying then is that for such devices, you
>> should not be able to statically assign them; you are only allowed to
>> hotplug them.
>>
>> If you want to statically assign such a device, then libxl *should* try
>> to make the RMRR if possible, but shouldn't fail if it can't; and, it
>> needs to tell Xen not to fail the assignment when setting up the domain.
>>
>> For that purpose, adding "rmrr=try" to the pci config spec makes the
>> most sense, doesn't it?
>>
>> Or am I missing something?
> 
> No, you're right. The model just is a little more complicated: The
> rmrr = [] settings need to be combined with the statically assigned
> devices' pci = [] settings. What will get most problematic is if you
> want rmrr = [ "host,check=force" ] but then make an exception for
> a statically assigned device (like a USB controller).

Yes, that's a policy question we'll have to think about; but that can
probably wait until the patches are posted.

Just to be clear -- what we're talking about here is that at the
do_domain_create() level (called by libxl_domain_create_new()), it will
take a list of pci devices, and the rmrr list above (including "host"
and individual ranges), and generate a list of RMRRs to pass to the
lower layer.  The lower layer will simply see the range, and a "force /
no force" flag, and behave appropriately.  The determination of which
RMRRs to force will be done at the domain_create level.

Is that about right?

 -George