| Message ID | 57C56AD8020000780010A125@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/30/2016 12:15 PM, Jan Beulich wrote: > Within compat_memory_op() this needs to be placed in the first switch() > statement, or it ends up being dead code (as that first switch() has a > default case chaining to compat_arch_memory_op()). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Compile tested only. Looks good to me, and I can try to actually test the code if there's a way to do it without having to install a 32-bit dom0. Is there a hypervisor command line argument that can help with testing? Thanks, Razvan
>>> On 30.08.16 at 11:46, <rcojocaru@bitdefender.com> wrote: > On 08/30/2016 12:15 PM, Jan Beulich wrote: >> Within compat_memory_op() this needs to be placed in the first switch() >> statement, or it ends up being dead code (as that first switch() has a >> default case chaining to compat_arch_memory_op()). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> Compile tested only. > > Looks good to me, and I can try to actually test the code if there's a > way to do it without having to install a 32-bit dom0. > > Is there a hypervisor command line argument that can help with testing? I'm afraid this can only be tested by invoking the operation from a 32-bit domain. I don't think that you'd need a Dom0 for that though: For a DomU, error codes should differ between old and new code (-ENOSYS vs -EINVAL afaics). Jan
On 08/30/2016 01:05 PM, Jan Beulich wrote: >>>> On 30.08.16 at 11:46, <rcojocaru@bitdefender.com> wrote: >> On 08/30/2016 12:15 PM, Jan Beulich wrote: >>> Within compat_memory_op() this needs to be placed in the first switch() >>> statement, or it ends up being dead code (as that first switch() has a >>> default case chaining to compat_arch_memory_op()). >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> --- >>> Compile tested only. >> >> Looks good to me, and I can try to actually test the code if there's a >> way to do it without having to install a 32-bit dom0. >> >> Is there a hypervisor command line argument that can help with testing? > > I'm afraid this can only be tested by invoking the operation from a > 32-bit domain. I don't think that you'd need a Dom0 for that though: > For a DomU, error codes should differ between old and new code > (-ENOSYS vs -EINVAL afaics). I've set up a 32-bit Ubuntu-based PV DomU: # uname -a Linux ubuntu 4.2.0-27-generic #32~14.04.1-Ubuntu SMP Fri Jan 22 15:32:27 UTC 2016 i686 i686 i686 GNU/Linux Without your patch, I get this when trying to run the mem_access.c test on a 64-bit domain running in Dom0: # ./xen-access 4 write xenaccess init max_gpfn = 10fc01 starting write 4 Error -1 setting default mem access type xenaccess exit code -1 With the patch applied, it's working as expected. Thank you for the patch! Tested-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Thanks, Razvan
On 30/08/16 10:15, Jan Beulich wrote: > Within compat_memory_op() this needs to be placed in the first switch() > statement, or it ends up being dead code (as that first switch() has a > default case chaining to compat_arch_memory_op()). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/compat/memory.c +++ b/xen/common/compat/memory.c @@ -320,6 +320,11 @@ int compat_memory_op(unsigned int cmd, X break; } + case XENMEM_access_op: + return mem_access_memop(cmd, + guest_handle_cast(compat, + xen_mem_access_op_t)); + case XENMEM_get_vnumainfo: { enum XLAT_vnuma_topology_info_vdistance vdistance = @@ -495,10 +500,6 @@ int compat_memory_op(unsigned int cmd, X break; } - case XENMEM_access_op: - rc = mem_access_memop(cmd, guest_handle_cast(compat, xen_mem_access_op_t)); - break; - case XENMEM_add_to_physmap_batch: start_extent = end_extent; break;