On 21/12/2022 1:24 pm, Jan Beulich wrote: > Especially with struct shadow_domain and struct hap_domain not living in > a union inside struct paging_domain, let's avoid the duplication: The > fields are named and used in identical ways, and only one of HAP or > shadow can be in use for a domain. This then also renders involved > expressions slightly more legible. > > Signed-off-by: Jan Beulich <[email protected]>
Reviewed-by: Andrew Cooper <[email protected]>, with two minor suggestions. > --- > Quite likely more folding of code is possible with this. For example > {hap,shadow}_set_allocation() are now yet more similar than they already > were. TBH, I think it wants to go further still. paging_mempool was intentionally common because all HAP architectures need it, and I really don't want to be playing XSA-410 with every new architecture. But this is fine to defer until further work. > --- a/xen/arch/x86/include/asm/domain.h > +++ b/xen/arch/x86/include/asm/domain.h > @@ -179,10 +173,6 @@ struct shadow_vcpu { > /* hardware assisted paging */ > /************************************************/ > struct hap_domain { > - struct page_list_head freelist; > - unsigned int total_pages; /* number of pages allocated */ > - unsigned int free_pages; /* number of pages on freelists */ > - unsigned int p2m_pages; /* number of pages allocated to p2m */ > }; Do we want to drop hap_domain? I can't forsee needing to put anything back into it. > --- a/xen/arch/x86/mm/paging.c > +++ b/xen/arch/x86/mm/paging.c > @@ -979,17 +980,17 @@ int __init paging_set_allocation(struct > > int arch_get_paging_mempool_size(struct domain *d, uint64_t *size) > { > - int rc; > + unsigned long pages; > > if ( is_pv_domain(d) ) /* TODO: Relax in due course */ > return -EOPNOTSUPP; > > - if ( hap_enabled(d) ) > - rc = hap_get_allocation_bytes(d, size); > - else > - rc = shadow_get_allocation_bytes(d, size); > + pages = d->arch.paging.total_pages; > + pages += d->arch.paging.p2m_pages; Any chance I can talk you into having a second space before the =, so we get: pages = d->arch.paging.total_pages; pages += d->arch.paging.p2m_pages; nicely aligned vertically? ~Andrew
