Re: [Xen-devel] [PATCH RFC 29/31] x86/pv: Provide custom cpumasks for PV domains

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.01.16 at 15:24, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/01/16 09:56, Jan Beulich wrote:
>>>>> On 16.12.15 at 22:24, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/cpu/amd.c
>>> +++ b/xen/arch/x86/cpu/amd.c
>>> @@ -203,7 +203,9 @@ static void __init noinline probe_masking_msrs(void)
>>>  void amd_ctxt_switch_levelling(const struct domain *nextd)
>>>  {
>>>     struct cpumasks *these_masks = &this_cpu(cpumasks);
>>> -   const struct cpumasks *masks = &cpumask_defaults;
>>> +   const struct cpumasks *masks =
>>> +            (nextd && is_pv_domain(nextd) && nextd->arch.pv_domain.masks)
>>> +            ? nextd->arch.pv_domain.masks : &cpumask_defaults;
>> Can nextd really ever be NULL here?
> 
> Yes, when using this function to set the defaults in the first place
> during AP bringup.

Ah, I then didn't spot this second use.

>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -578,6 +578,12 @@ int arch_domain_create(struct domain *d, unsigned int 
>>> domcr_flags,
>>>              goto fail;
>>>          clear_page(d->arch.pv_domain.gdt_ldt_l1tab);
>>>  
>>> +        d->arch.pv_domain.masks = xmalloc(struct cpumasks);
>>> +        if ( !d->arch.pv_domain.masks )
>>> +            goto fail;
>>> +        memcpy(d->arch.pv_domain.masks, &cpumask_defaults,
>>> +               sizeof(*d->arch.pv_domain.masks));
>> Structure assignment, to make the thing type safe?
>>
>> Also there's a change missing to the cleanup code after the "fail"
>> label.
> 
> What change are you thinking of?  I suppose an xfree() wouldn't go amis,
> to prevent a problem for whomever introduces a new failure path, but I
> don't see a bug in the code as-is.

I don't understand this second sentence. It's the missing addition
of a matching xfree() that my comment was about.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel