> -----Original Message----- > From: Andrew Cooper <[email protected]> > Sent: 24 September 2020 11:58 > To: [email protected]; 'Xen-devel' <[email protected]> > Cc: 'George Dunlap' <[email protected]>; 'Ian Jackson' > <[email protected]>; 'Jan Beulich' > <[email protected]>; 'Stefano Stabellini' <[email protected]>; 'Wei Liu' > <[email protected]>; 'Julien > Grall' <[email protected]>; 'Michał Leszczyński' <[email protected]>; > 'Hubert Jasudowicz' > <[email protected]>; 'Tamas K Lengyel' <[email protected]> > Subject: Re: [PATCH v2 04/11] xen/memory: Fix acquire_resource size semantics > > On 24/09/2020 11:06, Paul Durrant wrote: > >> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > >> index d1cfc8fb4a..e82307bdae 100644 > >> --- a/xen/arch/x86/mm.c > >> +++ b/xen/arch/x86/mm.c > >> @@ -4591,6 +4591,26 @@ int xenmem_add_to_physmap_one( > >> return rc; > >> } > >> > >> +unsigned int arch_resource_max_frames( > >> + struct domain *d, unsigned int type, unsigned int id) > >> +{ > >> + unsigned int nr = 0; > >> + > >> + switch ( type ) > >> + { > >> +#ifdef CONFIG_HVM > >> + case XENMEM_resource_ioreq_server: > >> + if ( !is_hvm_domain(d) ) > >> + break; > >> + /* One frame for the buf-ioreq ring, and one frame per 128 vcpus. > >> */ > >> + nr = 1 + DIV_ROUND_UP(d->max_vcpus * sizeof(struct ioreq), > >> PAGE_SIZE); > > The buf-ioreq ring is optional > > Yes, but it's position within the resource, and effect on the position > of the ioreq page(s), is not.
Oh yes, I was forgetting that this is fixed so... Reviewed-by: Paul Durrant <[email protected]> > > > so a caller using this value may still get a resource acquisition failure > > unless the id is used to > actually look up and check the ioreq server in question for the actual > maximum. > > Yes, but that is potentially true of *any* acquisition attempt, even if > the id matches, because maybe someone else has destroyed the ioreq > server, or the domain, in the meantime. > > > What we have is an mmap() where the caller needs to know to not try and > map page 0 for an ioreq server where buf-ioreq doesn't exist. > > This is a direct consequence of: > > #define XENMEM_resource_ioreq_server_frame_bufioreq 0 > #define XENMEM_resource_ioreq_server_frame_ioreq(n) (1 + (n)) > > and in practice, what a qemu/demu/other needs to do to map just the > ioreq frames (in a manner compatible with >127 vcpu HVM domains) is to > query the resource size and map size-1 pages from offset 1. Yes. Paul > > ~Andrew
