* hvmemul_rep_movs() vs MMIO
@ 2013-09-20 11:46 Jan Beulich
2013-09-20 12:05 ` Tim Deegan
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2013-09-20 11:46 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel
Tim,
was it really intended for "x86/hvm: use unlocked p2m lookups in
hvmemul_rep_movs()" to special case p2m_mmio_dm but not
p2m_mmio_direct?
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: hvmemul_rep_movs() vs MMIO
2013-09-20 11:46 hvmemul_rep_movs() vs MMIO Jan Beulich
@ 2013-09-20 12:05 ` Tim Deegan
2013-09-20 12:41 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Tim Deegan @ 2013-09-20 12:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
At 12:46 +0100 on 20 Sep (1379681171), Jan Beulich wrote:
> Tim,
>
> was it really intended for "x86/hvm: use unlocked p2m lookups in
> hvmemul_rep_movs()" to special case p2m_mmio_dm but not
> p2m_mmio_direct?
Hmm. It certainly doesn't seem to handle that case very well now, but
I'm not sure the code before was any better. AFAICT it would have
passed mmio_direct accesses to hvmemul_do_mmio(), which would send them
to qemu.
Tim.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: hvmemul_rep_movs() vs MMIO
2013-09-20 12:05 ` Tim Deegan
@ 2013-09-20 12:41 ` Jan Beulich
2013-09-20 12:47 ` Tim Deegan
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2013-09-20 12:41 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel
>>> On 20.09.13 at 14:05, Tim Deegan <tim@xen.org> wrote:
> At 12:46 +0100 on 20 Sep (1379681171), Jan Beulich wrote:
>> Tim,
>>
>> was it really intended for "x86/hvm: use unlocked p2m lookups in
>> hvmemul_rep_movs()" to special case p2m_mmio_dm but not
>> p2m_mmio_direct?
>
> Hmm. It certainly doesn't seem to handle that case very well now, but
> I'm not sure the code before was any better. AFAICT it would have
> passed mmio_direct accesses to hvmemul_do_mmio(), which would send them
> to qemu.
Hmm, wait - if MMIO of a passed through device, other than for
its port I/O, doesn't get intercepted at all, but instead gets taken
care of by there being a valid gfn->mfn translation in place, then
indeed before and after your change things aren't handled well.
Perhaps we should bail from there if either side is p2m_mmio_direct
as well as if both sides are p2m_mmio_dm:
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -799,6 +799,10 @@ static int hvmemul_rep_movs(
(void) get_gfn_query_unlocked(current->domain, sgpa >> PAGE_SHIFT, &sp2mt);
(void) get_gfn_query_unlocked(current->domain, dgpa >> PAGE_SHIFT, &dp2mt);
+ if ( sp2mt == p2m_mmio_direct || dp2mt == p2m_mmio_direct ||
+ (sp2mt == p2m_mmio_dm && dp2mt == p2m_mmio_dm) )
+ return X86EMUL_UNHANDLEABLE;
+
if ( sp2mt == p2m_mmio_dm )
return hvmemul_do_mmio(
sgpa, reps, bytes_per_rep, dgpa, IOREQ_READ, df, NULL);
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: hvmemul_rep_movs() vs MMIO
2013-09-20 12:41 ` Jan Beulich
@ 2013-09-20 12:47 ` Tim Deegan
2013-09-20 12:53 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Tim Deegan @ 2013-09-20 12:47 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
At 13:41 +0100 on 20 Sep (1379684515), Jan Beulich wrote:
> >>> On 20.09.13 at 14:05, Tim Deegan <tim@xen.org> wrote:
> > At 12:46 +0100 on 20 Sep (1379681171), Jan Beulich wrote:
> >> Tim,
> >>
> >> was it really intended for "x86/hvm: use unlocked p2m lookups in
> >> hvmemul_rep_movs()" to special case p2m_mmio_dm but not
> >> p2m_mmio_direct?
> >
> > Hmm. It certainly doesn't seem to handle that case very well now, but
> > I'm not sure the code before was any better. AFAICT it would have
> > passed mmio_direct accesses to hvmemul_do_mmio(), which would send them
> > to qemu.
>
> Hmm, wait - if MMIO of a passed through device, other than for
> its port I/O, doesn't get intercepted at all, but instead gets taken
> care of by there being a valid gfn->mfn translation in place, then
> indeed before and after your change things aren't handled well.
> Perhaps we should bail from there if either side is p2m_mmio_direct
> as well as if both sides are p2m_mmio_dm:
Sounds OK to me. Clearly mmio-mmio string operations aren't something
we need to make go fast. :)
Reviewed-by: Tim Deegan <tim@xen.org>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -799,6 +799,10 @@ static int hvmemul_rep_movs(
> (void) get_gfn_query_unlocked(current->domain, sgpa >> PAGE_SHIFT, &sp2mt);
> (void) get_gfn_query_unlocked(current->domain, dgpa >> PAGE_SHIFT, &dp2mt);
>
> + if ( sp2mt == p2m_mmio_direct || dp2mt == p2m_mmio_direct ||
> + (sp2mt == p2m_mmio_dm && dp2mt == p2m_mmio_dm) )
> + return X86EMUL_UNHANDLEABLE;
> +
> if ( sp2mt == p2m_mmio_dm )
> return hvmemul_do_mmio(
> sgpa, reps, bytes_per_rep, dgpa, IOREQ_READ, df, NULL);
>
> Jan
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: hvmemul_rep_movs() vs MMIO
2013-09-20 12:47 ` Tim Deegan
@ 2013-09-20 12:53 ` Jan Beulich
0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2013-09-20 12:53 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel
>>> On 20.09.13 at 14:47, Tim Deegan <tim@xen.org> wrote:
> At 13:41 +0100 on 20 Sep (1379684515), Jan Beulich wrote:
>> >>> On 20.09.13 at 14:05, Tim Deegan <tim@xen.org> wrote:
>> > At 12:46 +0100 on 20 Sep (1379681171), Jan Beulich wrote:
>> >> Tim,
>> >>
>> >> was it really intended for "x86/hvm: use unlocked p2m lookups in
>> >> hvmemul_rep_movs()" to special case p2m_mmio_dm but not
>> >> p2m_mmio_direct?
>> >
>> > Hmm. It certainly doesn't seem to handle that case very well now, but
>> > I'm not sure the code before was any better. AFAICT it would have
>> > passed mmio_direct accesses to hvmemul_do_mmio(), which would send them
>> > to qemu.
>>
>> Hmm, wait - if MMIO of a passed through device, other than for
>> its port I/O, doesn't get intercepted at all, but instead gets taken
>> care of by there being a valid gfn->mfn translation in place, then
>> indeed before and after your change things aren't handled well.
>> Perhaps we should bail from there if either side is p2m_mmio_direct
>> as well as if both sides are p2m_mmio_dm:
>
> Sounds OK to me. Clearly mmio-mmio string operations aren't something
> we need to make go fast. :)
Who knows: Frame buffer internal moves might e.g. be desirable
to get accelerated. But if we want to do such, be need to honor
the original access size at the very least.
> Reviewed-by: Tim Deegan <tim@xen.org>
Actually the final patch is to be bigger - we need to do something
similar in rep_ins and rep_outs. Will send the whole thing in a few
minutes (after smoke testing).
Jan
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -799,6 +799,10 @@ static int hvmemul_rep_movs(
>> (void) get_gfn_query_unlocked(current->domain, sgpa >> PAGE_SHIFT, &sp2mt);
>> (void) get_gfn_query_unlocked(current->domain, dgpa >> PAGE_SHIFT, &dp2mt);
>>
>> + if ( sp2mt == p2m_mmio_direct || dp2mt == p2m_mmio_direct ||
>> + (sp2mt == p2m_mmio_dm && dp2mt == p2m_mmio_dm) )
>> + return X86EMUL_UNHANDLEABLE;
>> +
>> if ( sp2mt == p2m_mmio_dm )
>> return hvmemul_do_mmio(
>> sgpa, reps, bytes_per_rep, dgpa, IOREQ_READ, df, NULL);
>>
>> Jan
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-20 12:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-20 11:46 hvmemul_rep_movs() vs MMIO Jan Beulich
2013-09-20 12:05 ` Tim Deegan
2013-09-20 12:41 ` Jan Beulich
2013-09-20 12:47 ` Tim Deegan
2013-09-20 12:53 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).