[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v13 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: 24 November 2017 09:58 > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx> > Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper > <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap > <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu > <wei.liu2@xxxxxxxxxx>; StefanoStabellini <sstabellini@xxxxxxxxxx>; xen- > devel@xxxxxxxxxxxxxxxxxxxx; Konrad Rzeszutek Wilk > <konrad.wilk@xxxxxxxxxx>; Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>; Tim > (Xen.org) <tim@xxxxxxx> > Subject: RE: [PATCH v13 05/11] x86/mm: add HYPERVISOR_memory_op to > acquire guest resources > > >>> On 24.11.17 at 10:36, <Paul.Durrant@xxxxxxxxxx> wrote: > >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > >> Sent: 23 November 2017 16:42 > >> >>> On 30.10.17 at 18:48, <paul.durrant@xxxxxxxxxx> wrote: > >> > + if ( !paging_mode_translate(currd) ) > >> > + { > >> > + if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) ) > >> > + rc = -EFAULT; > >> > + } > >> > + else > >> > + { > >> > + xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)]; > >> > + unsigned int i; > >> > + > >> > + rc = -EFAULT; > >> > + if ( copy_from_guest(gfn_list, xmar.frame_list, xmar.nr_frames) > >> > ) > >> > + goto out; > >> > >> This will result in requests with nr_frames being zero to fail with > >> -EFAULT afaict. Let's please have such no-op requests succeed. > > > > Oh, I was labouring under the assumption that a copy_from_guest() with a > > zero count would succeed... I should have checked. > > I don't think it would fail. The problem is that you won't enter ... > > >> > + for ( i = 0; i < xmar.nr_frames; i++ ) > >> > + { > > ... the body of this loop then, and hence rc will remain at -EFAULT. > > >> > + rc = set_foreign_p2m_entry(currd, gfn_list[i], > >> > + _mfn(mfn_list[i])); > >> > + if ( rc ) > >> > + { > >> > + /* > >> > + * Make sure rc is -EIO for any iteration other than > >> > + * the first. > >> > + */ > >> > + rc = (i != 0) ? -EIO : rc; > >> > >> Along the lines of what I've said above, "!=0" could be dropped > >> here, too. > > > > The reason for the != 0 here is make the failure explicitly EIO for the case > > where at least one iteration has successfully set_foreign_p2m_entry() but > a > > subsequent one has failed, which matches what is stated in the header. > > (Specifically it means that, on ARM, the -EOPNOTSUPP from > > set_foreign_p2m_entry() will get back to the caller). > > I'm not asking for the condition to be removed, just for it to be > simplified to > > rc = i ? -EIO : rc; > Ah, ok. That's fine then. Thanks, Paul > Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |