On 09/06/2018 04:05 PM, Jan Beulich wrote:
>>>> On 04.09.18 at 18:15, <[email protected]> wrote:
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>> /* Init the datastructures for later use by the p2m code */
>> static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>> {
>> - unsigned int i;
>> int ret = 0;
>> +#ifdef CONFIG_HVM
>> + unsigned int i;
>> +#endif
>>
>> mm_rwlock_init(&p2m->lock);
>> - mm_lock_init(&p2m->pod.lock);
>> INIT_LIST_HEAD(&p2m->np2m_list);
>> INIT_PAGE_LIST_HEAD(&p2m->pages);
>> +
>> +#ifdef CONFIG_HVM
>> + mm_lock_init(&p2m->pod.lock);
>> INIT_PAGE_LIST_HEAD(&p2m->pod.super);
>> INIT_PAGE_LIST_HEAD(&p2m->pod.single);
>> +#endif
>>
>> p2m->domain = d;
>> p2m->default_access = p2m_access_rwx;
>> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct
>> p2m_domain *p2m)
>> p2m->np2m_base = P2M_BASE_EADDR;
>> p2m->np2m_generation = 0;
>>
>> +#ifdef CONFIG_HVM
>> for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
>> p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
>> +#endif
>
> I wonder whether all of this wouldn't better go into p2m_pod_init()
> (or alike), to limit/avoid the #ifdef-ary here.
The cover letter for this series did say the initial goal was just to
make it build; and cleaning up could be done later. Adding
p2m_pod_init() would be better long term, I think, but I'm also happy
with leaving if #ifdef's for now and cleaning them up later.
>
>> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
>> P2M_PRINTK("p2m audit starts\n");
>>
>> p2m_lock(p2m);
>> +
>> +#ifdef CONFIG_HVM
>> pod_lock(p2m);
>> +#endif
>>
>> if (p2m->audit_p2m)
>> pmbad = p2m->audit_p2m(p2m);
>> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
>> }
>> spin_unlock(&d->page_alloc_lock);
>>
>> +#ifdef CONFIG_HVM
>> pod_unlock(p2m);
>> +#endif
>
> Perhaps better make them inline stubs? Otoh - isn't the whole
> function useful for HVM only?
Oh yes -- now that we've gotten rid of full translate guests for PV, I
guess so.
>
>> --- a/xen/common/vm_event.c
>> +++ b/xen/common/vm_event.c
>> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct
>> xen_domctl_vm_event_op *vec,
>> {
>> case XEN_VM_EVENT_ENABLE:
>> {
>> +#ifdef CONFIG_HVM
>> struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +#endif
>>
>> rc = -EOPNOTSUPP;
>> /* hvm fixme: p2m_is_foreign types need addressing */
>> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct
>> xen_domctl_vm_event_op *vec,
>> if ( unlikely(need_iommu(d)) )
>> break;
>>
>> +#ifdef CONFIG_HVM
>> rc = -EXDEV;
>> /* Disallow paging in a PoD guest */
>> if ( p2m->pod.entry_count )
>> break;
>> +#endif
>
> Perhaps simply ditch the local variable?
That seems like a good idea.
Everything else looks good to me.
-George
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
