Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-11 Thread Avi Kivity
On 12/08/2011 05:37 PM, Sasha Levin wrote:
 On Thu, 2011-12-08 at 20:52 +1030, Rusty Russell wrote:
  Here's the patch series I ended up with.  I haven't coded up the QEMU
  side yet, so no idea if the new driver works.
  
  Questions:
  (1) Do we win from separating ISR, NOTIFY and COMMON?
  (2) I used a u8 bar; should I use a bir and pack it instead?  BIR
  seems a little obscure (noone else in the kernel source seems to
  refer to it).

 I started implementing it for KVM tools, when I noticed a strange thing:
 my vq creating was failing because the driver was reading a value other
 than 0 from the address field of a new vq, and failing.

 I've added simple prints in the usermode code, and saw the following
 ordering:

 1. queue select vq 0
 2. queue read address (returns 0 - new vq)
 3. queue write address (good address of vq)
 4. queue read address (returns !=0, fails)
 4. queue select vq 1

 From that I understood that the ordering is wrong, the driver was trying
 to read address before selecting the correct vq.

 At that point, I've added simple prints to the driver. Initially it
 looked as follows:

   iowrite16(index, vp_dev-common-queue_select);

   switch (ioread64(vp_dev-common-queue_address)) {
   [...]
   };

 So I added prints before the iowrite16() and after the ioread64(), and
 saw that while the driver prints were ordered, the device ones weren't:

   [1.264052] before iowrite index=1
   kvmtool: net returning pfn (vq=0): 310706176
   kvmtool: queue selected: 1
   [1.264890] after ioread index=1

 Suspecting that something was wrong with ordering, I've added a print
 between the iowrite and the ioread, and it finally started working well.

 Which leads me to the question: Are MMIO vs MMIO reads/writes not
 ordered?

mmios are strictly ordered.

Perhaps your printfs are reordered by buffering?  Are they from
different threads?  Are you using coalesced mmio (which is still
strictly ordered, if used correctly)?

-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-11 Thread Sasha Levin
On Sun, 2011-12-11 at 11:05 +0200, Avi Kivity wrote:
 mmios are strictly ordered.
 
 Perhaps your printfs are reordered by buffering?  Are they from
 different threads?  Are you using coalesced mmio (which is still
 strictly ordered, if used correctly)? 

I print the queue_selector and queue_address in the printfs, even if
printfs were reordered they would be printing the data right, unlike
they do now. It's the data in the printfs that matters, not their order.

Same vcpu thread with both accesses.

Not using coalesced mmio.

-- 

Sasha.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-11 Thread Michael S. Tsirkin
On Sun, Dec 11, 2011 at 12:03:52PM +0200, Sasha Levin wrote:
 On Sun, 2011-12-11 at 11:05 +0200, Avi Kivity wrote:
  mmios are strictly ordered.
  
  Perhaps your printfs are reordered by buffering?  Are they from
  different threads?  Are you using coalesced mmio (which is still
  strictly ordered, if used correctly)? 
 
 I print the queue_selector and queue_address in the printfs, even if
 printfs were reordered they would be printing the data right, unlike
 they do now. It's the data in the printfs that matters, not their order.
 
 Same vcpu thread with both accesses.
 
 Not using coalesced mmio.

Not sure why this would matter, but is the BAR a prefetcheable one?
Rusty's patch uses pci_iomap which maps a prefetcheable BAR
as cacheable.


 -- 
 
 Sasha.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-11 Thread Michael S. Tsirkin
On Thu, Dec 08, 2011 at 05:37:37PM +0200, Sasha Levin wrote:
 On Thu, 2011-12-08 at 20:52 +1030, Rusty Russell wrote:
  Here's the patch series I ended up with.  I haven't coded up the QEMU
  side yet, so no idea if the new driver works.
  
  Questions:
  (1) Do we win from separating ISR, NOTIFY and COMMON?
  (2) I used a u8 bar; should I use a bir and pack it instead?  BIR
  seems a little obscure (noone else in the kernel source seems to
  refer to it).
 
 I started implementing it for KVM tools, when I noticed a strange thing:
 my vq creating was failing because the driver was reading a value other
 than 0 from the address field of a new vq, and failing.
 
 I've added simple prints in the usermode code, and saw the following
 ordering:
 
 1. queue select vq 0
 2. queue read address (returns 0 - new vq)
 3. queue write address (good address of vq)
 4. queue read address (returns !=0, fails)
 4. queue select vq 1
 
 From that I understood that the ordering is wrong, the driver was trying
 to read address before selecting the correct vq.
 
 At that point, I've added simple prints to the driver. Initially it
 looked as follows:
 
   iowrite16(index, vp_dev-common-queue_select);
 
   switch (ioread64(vp_dev-common-queue_address)) {
   [...]
   };
 
 So I added prints before the iowrite16() and after the ioread64(), and
 saw that while the driver prints were ordered, the device ones weren't:
 
   [1.264052] before iowrite index=1
   kvmtool: net returning pfn (vq=0): 310706176
   kvmtool: queue selected: 1
   [1.264890] after ioread index=1
 
 Suspecting that something was wrong with ordering, I've added a print
 between the iowrite and the ioread, and it finally started working well.
 
 Which leads me to the question: Are MMIO vs MMIO reads/writes not
 ordered?

First, I'd like to answer your questions from the PCI side.
Look for PCI rules in the PCI spec.
You will notices that a write is required to be able to
pass a read request. It might also pass read completion.
A read request will not pass a write request.
There's more or less no ordering between different types of transactions
(memory versus io/configuration).

That's wrt to the question you asked.

But this is not your setup: you have a single vcpu so
you will not initiate a write (select vq) until you get
a read completion.

So what you are really describing is this setup: guest reads a value,
gets the response, then writes out another one, and kvm tool reports the
write before the read.


 -- 
 
 Sasha.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-11 Thread Sasha Levin
On Sun, 2011-12-11 at 14:30 +0200, Michael S. Tsirkin wrote:
 On Sun, Dec 11, 2011 at 12:03:52PM +0200, Sasha Levin wrote:
  On Sun, 2011-12-11 at 11:05 +0200, Avi Kivity wrote:
   mmios are strictly ordered.
   
   Perhaps your printfs are reordered by buffering?  Are they from
   different threads?  Are you using coalesced mmio (which is still
   strictly ordered, if used correctly)? 
  
  I print the queue_selector and queue_address in the printfs, even if
  printfs were reordered they would be printing the data right, unlike
  they do now. It's the data in the printfs that matters, not their order.
  
  Same vcpu thread with both accesses.
  
  Not using coalesced mmio.
 
 Not sure why this would matter, but is the BAR a prefetcheable one?
 Rusty's patch uses pci_iomap which maps a prefetcheable BAR
 as cacheable.

Wasn't defined as prefetchable, but I'm seeing same thing with or
without it.

-- 

Sasha.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-11 Thread Sasha Levin
On Sun, 2011-12-11 at 14:47 +0200, Michael S. Tsirkin wrote:
 First, I'd like to answer your questions from the PCI side.
 Look for PCI rules in the PCI spec.
 You will notices that a write is required to be able to
 pass a read request. It might also pass read completion.
 A read request will not pass a write request.
 There's more or less no ordering between different types of transactions
 (memory versus io/configuration).
 
 That's wrt to the question you asked.
 
 But this is not your setup: you have a single vcpu so
 you will not initiate a write (select vq) until you get
 a read completion.
 
 So what you are really describing is this setup: guest reads a value,
 gets the response, then writes out another one, and kvm tool reports the
 write before the read. 

No, it's exactly the opposite. Guest writes a value first and then reads
one (writes queue_select and reads queue_address) and kvm tool reporting
the read before the write.

I must add here that the kvm tool doesn't do anything fancy with simple
IO/MMIO. Theres no thread games or anything similar there. The vcpu
thread is doing all the IO/MMIO work.

-- 

Sasha.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-10 Thread Sasha Levin
On Fri, 2011-12-09 at 16:47 +1030, Rusty Russell wrote:
 On Thu, 08 Dec 2011 17:37:37 +0200, Sasha Levin levinsasha...@gmail.com 
 wrote:
  Which leads me to the question: Are MMIO vs MMIO reads/writes not
  ordered?
 
 That seems really odd, especially being repeatable.

Happens every single time. Can't be a coincidence.

I even went into paranoia mode and made sure that both IO requests come
from the same vcpu.

Another weird thing I've noticed is that mb() doesn't fix it, while if I
replace the mb() with a printk() it works well.

 BTW, that's an address, not a pfn now.

Fixed :)

-- 

Sasha.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-09 Thread Rusty Russell
On Thu, 08 Dec 2011 17:37:37 +0200, Sasha Levin levinsasha...@gmail.com wrote:
 Which leads me to the question: Are MMIO vs MMIO reads/writes not
 ordered?

That seems really odd, especially being repeatable.

BTW, that's an address, not a pfn now.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/11] RFC: PCI using capabilitities

2011-12-08 Thread Rusty Russell
Here's the patch series I ended up with.  I haven't coded up the QEMU
side yet, so no idea if the new driver works.

Questions:
(1) Do we win from separating ISR, NOTIFY and COMMON?
(2) I used a u8 bar; should I use a bir and pack it instead?  BIR
seems a little obscure (noone else in the kernel source seems to
refer to it).

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-08 Thread Sasha Levin
Rusty, I can't find the actual patches, could you verify that they were
indeed sent?

On Thu, 2011-12-08 at 20:52 +1030, Rusty Russell wrote:
 Here's the patch series I ended up with.  I haven't coded up the QEMU
 side yet, so no idea if the new driver works.
 
 Questions:
 (1) Do we win from separating ISR, NOTIFY and COMMON?

By separating ISR, NOTIFY and COMMON we can place ISR and NOTIFY in PIO
and COMMON in MMIO. This gives us the benefit of having the small data
path use fast PIO, while big config path can use MMIO.

 (2) I used a u8 bar; should I use a bir and pack it instead?  BIR
 seems a little obscure (noone else in the kernel source seems to
 refer to it).

BIR is a concept from the PCI spec, but it was only used for MSI-X. I
don't expect to see it all around the kernel source.

-- 

Sasha.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/11] RFC: PCI using capabilitities

2011-12-08 Thread Sasha Levin
On Thu, 2011-12-08 at 20:52 +1030, Rusty Russell wrote:
 Here's the patch series I ended up with.  I haven't coded up the QEMU
 side yet, so no idea if the new driver works.
 
 Questions:
 (1) Do we win from separating ISR, NOTIFY and COMMON?
 (2) I used a u8 bar; should I use a bir and pack it instead?  BIR
 seems a little obscure (noone else in the kernel source seems to
 refer to it).

I started implementing it for KVM tools, when I noticed a strange thing:
my vq creating was failing because the driver was reading a value other
than 0 from the address field of a new vq, and failing.

I've added simple prints in the usermode code, and saw the following
ordering:

1. queue select vq 0
2. queue read address (returns 0 - new vq)
3. queue write address (good address of vq)
4. queue read address (returns !=0, fails)
4. queue select vq 1

From that I understood that the ordering is wrong, the driver was trying
to read address before selecting the correct vq.

At that point, I've added simple prints to the driver. Initially it
looked as follows:

iowrite16(index, vp_dev-common-queue_select);

switch (ioread64(vp_dev-common-queue_address)) {
[...]
};

So I added prints before the iowrite16() and after the ioread64(), and
saw that while the driver prints were ordered, the device ones weren't:

[1.264052] before iowrite index=1
kvmtool: net returning pfn (vq=0): 310706176
kvmtool: queue selected: 1
[1.264890] after ioread index=1

Suspecting that something was wrong with ordering, I've added a print
between the iowrite and the ioread, and it finally started working well.

Which leads me to the question: Are MMIO vs MMIO reads/writes not
ordered?

-- 

Sasha.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html