| Message ID | 1587789a-b0d6-6d18-99fc-a94bbea52d7b@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86emul: further work | expand |
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 05 May 2020 09:15 > To: xen-devel@lists.xenproject.org > Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monne > <roger.pau@citrix.com>; Paul Durrant <paul@xen.org> > Subject: [PATCH v8 06/12] x86/HVM: make hvmemul_blk() capable of handling r/o operations > > In preparation for handling e.g. FLDENV or {F,FX,X}RSTOR here as well. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org>
On 05/05/2020 09:15, Jan Beulich wrote: > In preparation for handling e.g. FLDENV or {F,FX,X}RSTOR here as well. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > v8: New (could be folded into "x86emul: support MOVDIR{I,64B} insns", > but would invalidate Paul's R-b there). > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -1453,7 +1453,7 @@ static int hvmemul_blk( > struct hvm_emulate_ctxt *hvmemul_ctxt = > container_of(ctxt, struct hvm_emulate_ctxt, ctxt); > unsigned long addr; > - uint32_t pfec = PFEC_page_present | PFEC_write_access; > + uint32_t pfec = PFEC_page_present; > int rc; > void *mapping = NULL; > > @@ -1462,6 +1462,9 @@ static int hvmemul_blk( > if ( rc != X86EMUL_OKAY || !bytes ) > return rc; > > + if ( x86_insn_is_mem_write(state, ctxt) ) > + pfec |= PFEC_write_access; > + For the instructions with two memory operands, it conflates the read-only side with the read-write side. ~Andrew
On 07.05.2020 22:34, Andrew Cooper wrote: > On 05/05/2020 09:15, Jan Beulich wrote: >> In preparation for handling e.g. FLDENV or {F,FX,X}RSTOR here as well. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> v8: New (could be folded into "x86emul: support MOVDIR{I,64B} insns", >> but would invalidate Paul's R-b there). >> >> --- a/xen/arch/x86/hvm/emulate.c >> +++ b/xen/arch/x86/hvm/emulate.c >> @@ -1453,7 +1453,7 @@ static int hvmemul_blk( >> struct hvm_emulate_ctxt *hvmemul_ctxt = >> container_of(ctxt, struct hvm_emulate_ctxt, ctxt); >> unsigned long addr; >> - uint32_t pfec = PFEC_page_present | PFEC_write_access; >> + uint32_t pfec = PFEC_page_present; >> int rc; >> void *mapping = NULL; >> >> @@ -1462,6 +1462,9 @@ static int hvmemul_blk( >> if ( rc != X86EMUL_OKAY || !bytes ) >> return rc; >> >> + if ( x86_insn_is_mem_write(state, ctxt) ) >> + pfec |= PFEC_write_access; >> + > > For the instructions with two memory operands, it conflates the > read-only side with the read-write side. "Conflates" is probably the wrong word? The bug you point out is really in x86_insn_is_mem_write(), in that so far it would return false for MOVDIR64B and ENQCMD{,S}. As a result I'll fix the issue there while, at the same time, folding this patch into "x86emul: support MOVDIR{I,64B} insns". Jan
--- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -1453,7 +1453,7 @@ static int hvmemul_blk( struct hvm_emulate_ctxt *hvmemul_ctxt = container_of(ctxt, struct hvm_emulate_ctxt, ctxt); unsigned long addr; - uint32_t pfec = PFEC_page_present | PFEC_write_access; + uint32_t pfec = PFEC_page_present; int rc; void *mapping = NULL; @@ -1462,6 +1462,9 @@ static int hvmemul_blk( if ( rc != X86EMUL_OKAY || !bytes ) return rc; + if ( x86_insn_is_mem_write(state, ctxt) ) + pfec |= PFEC_write_access; + if ( is_x86_system_segment(seg) ) pfec |= PFEC_implicit; else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
In preparation for handling e.g. FLDENV or {F,FX,X}RSTOR here as well. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v8: New (could be folded into "x86emul: support MOVDIR{I,64B} insns", but would invalidate Paul's R-b there).