string mmio/pio across page boundaries

string mmio/pio across page boundaries

[Jan Beulich]

When preparing arguments for send_mmio_req() or send_pio_req() in hvm
where a single iteration crosses a page boundary, the value to be written
is copied from guest memory, but I can't see where the value read is copied
to guest memory. Is this case simply missing?

Also, a long while back the logic to calculate when wrapping occurs was
changed with two subsequent patches. As I'm looking at the code now it
seems like
- only the first patch got applied to handle_mmio() (i.e., the logic is still
  broken
- neither patch got applied to {svm,vmx}_io_instruction().

Further, there is a comment in handle_mmio() now saying "We need to
make sure we advance to the point where the next request will be on a
different page.  If we're going down, that means advancing until one
byte before the start of the page, hence +1." This wasn't applied
similarly to {svm,vmx}_io_instruction(). And the actual code seems
wrong: If e.g. addr is on a page boundary, count will become zero. I
would think that the +1 must be outside of the division.

Was this intentional in some way? Otherwise, I'll prepare a patch to
address all of these.

Thanks, Jan

Re: string mmio/pio across page boundaries

[Jan Beulich]

>>> "Jan Beulich" <jbeulich@novell.com> 23.11.06 11:22 >>>
>When preparing arguments for send_mmio_req() or send_pio_req() in hvm
>where a single iteration crosses a page boundary, the value to be written
>is copied from guest memory, but I can't see where the value read is copied
>to guest memory. Is this case simply missing?

Additionally - how is ecx being updated in all these wrapping cases?

Jan

RE: string mmio/pio across page boundaries

[Li, Xin B]

>
>When preparing arguments for send_mmio_req() or send_pio_req() in hvm
>where a single iteration crosses a page boundary, the value to 
>be written is copied from guest memory, but I can't see where the value

>read is copied to guest memory. Is this case simply missing?
>

in send_mmio_req or send_pio_req, there is no request crossing a page
boundary, since it's already converted.


>Also, a long while back the logic to calculate when wrapping occurs was
>changed with two subsequent patches. As I'm looking at the code now it
>seems like - only the first patch got applied to handle_mmio() (i.e.,
the 
>logic is still broken - neither patch got applied to
{svm,vmx}_io_instruction().
>
>Further, there is a comment in handle_mmio() now saying "We need to
>make sure we advance to the point where the next request will be on a
>different page.  If we're going down, that means advancing until one
>byte before the start of the page, hence +1." This wasn't applied
>similarly to {svm,vmx}_io_instruction(). And the actual code seems
>wrong: If e.g. addr is on a page boundary, count will become zero. I
>would think that the +1 must be outside of the division.
>
>Was this intentional in some way? Otherwise, I'll prepare a patch to
>address all of these.
>

Are you using an old source tree, I don't see such code, and we have
sent a patch to clean that code before xen 3.0.3.
I think the logic is correct now.
-Xin

RE: string mmio/pio across page boundaries

[Li, Xin B]

>>
>
>Are you using an old source tree, I don't see such code, and we have
>sent a patch to clean that code before xen 3.0.3.

Oops, I'm not sure if the patch is in xen 3.0.3.
-Xin

RE: string mmio/pio across page boundaries

[Jan Beulich]

>>> "Li, Xin B" <xin.b.li@intel.com> 23.11.06 12:36 >>>
>>
>>When preparing arguments for send_mmio_req() or send_pio_req() in hvm
>>where a single iteration crosses a page boundary, the value to 
>>be written is copied from guest memory, but I can't see where the value
>>read is copied to guest memory. Is this case simply missing?
>>
>
>in send_mmio_req or send_pio_req, there is no request crossing a page
>boundary, since it's already converted.

I didn't say in these two functions, I said when preparing the arguments
for them. Would you look at this again, please? Similarly for the updating
of ecx that I asked about in a subsequent mail.

>Are you using an old source tree, I don't see such code, and we have
>sent a patch to clean that code before xen 3.0.3.
>I think the logic is correct now.

It isn't - I'm working on a 3.0.3 based tree right now. But yes, I see
that -unstable has this fixed. I should have checked...

Jan