On 03/01/2020 14:25, Jan Beulich wrote:
> On 17.12.2019 21:15, Andrew Cooper wrote:
>> --- a/tools/libxc/include/xc_dom.h
>> +++ b/tools/libxc/include/xc_dom.h
>> @@ -123,16 +123,12 @@ struct xc_dom_image {
>>
>> /* other state info */
>> uint32_t f_active[XENFEAT_NR_SUBMAPS];
>> +
>> /*
>> - * p2m_host maps guest physical addresses an offset from
>> - * rambase_pfn (see below) into gfns.
> The removal of this part of the comment and ...
>
>> - * For a pure PV guest this means that it maps GPFNs into MFNs for
>> - * a hybrid guest this means that it maps GPFNs to GPFNS.
>> - *
>> - * Note that the input is offset by rambase.
>> + * pv_p2m is specific to x86 PV guests, and maps GFNs to MFNs. It is
>> + * eventually copied into guest context.
>> */
>> - xen_pfn_t *p2m_host;
>> + xen_pfn_t *pv_p2m;
>>
>> /* physical memory
>> *
>> @@ -433,9 +429,12 @@ static inline xen_pfn_t xc_dom_p2m(struct xc_dom_image
>> *dom, xen_pfn_t pfn)
>> {
>> if ( xc_dom_translated(dom) )
>> return pfn;
>> - if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn +
>> dom->total_pages)
>> +
>> + /* x86 PV only now. */
>> + if ( pfn >= dom->total_pages )
>> return INVALID_MFN;
>> - return dom->p2m_host[pfn - dom->rambase_pfn];
>> +
>> + return dom->pv_p2m[pfn];
>> }
> ... the dropping of all uses of rambase_pfn here make it look
> like you're doing away with that concept altogether. Is this
> really correct?
Well - it is effectively dead code here, because of the
xc_dom_translated() in context above, and it being 0 outside of ARM.
The (nonzero) value is used by other bits of ARM code.
> If so, I guess you want to
> - say a word in this regard in the description, the more that
> you don't fully get rid of this (the structure field and
> some uses elsewhere remain),
> - drop/adjust the respective comment fragment next to the
> field a little further down in the structure.
The domain builder is made almost exclusively of bitrot, but I don't
have an ARM system to do any testing on. Given how fragile the code is,
I'm not comfortable doing speculative deletion of code I'm not certain
is unused.
>
>> @@ -1245,11 +1245,11 @@ static int meminit_pv(struct xc_dom_image *dom)
>> return -EINVAL;
>> }
>>
>> - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
>> - if ( dom->p2m_host == NULL )
>> + dom->pv_p2m = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
> Since you're touching the line anyway, perhaps better
> sizeof(*dom->pv_p2m)?
Will fix.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
