Re: [Xen-devel] [V8 PATCH 2/8] pvh dom0: construct_dom0 changes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.03.14 at 11:55, <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 03/27/2014 10:14 AM, Jan Beulich wrote:
>>>>> On 26.03.14 at 20:05, <george.dunlap@xxxxxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/hap/hap.c
>>> +++ b/xen/arch/x86/mm/hap/hap.c
>>> @@ -589,6 +589,21 @@ int hap_domctl(struct domain *d, 
>>> xen_domctl_shadow_op_t 
> *sc,
>>>        }
>>>    }
>>>    
>>> +void __init hap_set_pvh_alloc_for_dom0(struct domain *d,
>>> +                                       unsigned long num_pages)
>>> +{
>>> +    int rc;
>>> +    unsigned long memkb = num_pages * (PAGE_SIZE / 1024);
>>> +
>>> +    /* Copied from: libxl_get_required_shadow_memory() */
>>> +    memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
>>> +    num_pages = ( (memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
>>> +    paging_lock(d);
>>> +    rc = hap_set_allocation(d, num_pages, NULL);
>>> +    paging_unlock(d);
>>>
>>>
>>> The calculation for how many pages of shadow memory are needed logically
>>> belongs in domain_build.c, not hap.c.
>> Iirc it was me requesting it to be moved here, on the basis that the
>> calculation should match the one for other domains, and hence be
>> done alongside that for other domains. domain_build.c shouldn't
>> carry HAP-specific knowledge imo.
> 
> I thought that might be the case, which is why I apologized in advance 
> for not reading the previous thread.
> 
> The calculation should indeed match that done for other domains; 
> however, as the comment clearly says, this calculation is done in libxl, 
> not in Xen at all.  Everything done to build dom0 in Xen which 
> corresponds to things done by the toolstack to build a domU logically 
> belongs in domain_build.c.
> 
> Putting it in hap.c would be wrong in any case; what would you do when 
> we enable shadow mode for HAP dom0?

The function calls hap_set_allocation(), so it's already HAP-specific.
I guess that was the main motivation to ask for it to be put there
(including the fact that this function is presently static, and should
preferably remain so).

Jan


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