* [PATCH 2/4] kvm tools: Fix PCI probing
2011-07-28 9:01 [PATCH 1/4] kvm tools: Use GSI routing Sasha Levin
@ 2011-07-28 9:01 ` Sasha Levin
2011-07-28 9:31 ` Pekka Enberg
2011-07-28 9:01 ` [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback Sasha Levin
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2011-07-28 9:01 UTC (permalink / raw)
To: penberg; +Cc: kvm, mingo, asias.hejun, gorcunov, prasadjoshi124, Sasha Levin
PCI BAR probing is done in four steps:
1. Read address (and flags).
2. Mask BAR.
3. Read BAR again - Now the expected result is the size of the BAR.
4. Mask BAR with address.
So far, we have only took care of the first step. This means that the kernel
was using address as the size, causing a PCI allocation blunder.
This patch fixes the issue by passing a proper size after masking.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/include/kvm/pci.h | 1 +
tools/kvm/pci.c | 57 +++++++++++++++++++++++++++++++++++++++----
2 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/tools/kvm/include/kvm/pci.h b/tools/kvm/include/kvm/pci.h
index 6ad4426..a7532e3 100644
--- a/tools/kvm/include/kvm/pci.h
+++ b/tools/kvm/include/kvm/pci.h
@@ -51,5 +51,6 @@ struct pci_device_header {
void pci__init(void);
void pci__register(struct pci_device_header *dev, u8 dev_num);
+u32 pci_get_io_space_block(void);
#endif /* KVM__PCI_H */
diff --git a/tools/kvm/pci.c b/tools/kvm/pci.c
index a1ad8ba..799536e3 100644
--- a/tools/kvm/pci.c
+++ b/tools/kvm/pci.c
@@ -5,11 +5,23 @@
#include <assert.h>
#define PCI_MAX_DEVICES 256
+#define PCI_IO_SIZE 0x100
static struct pci_device_header *pci_devices[PCI_MAX_DEVICES];
static struct pci_config_address pci_config_address;
+/* This is within our PCI gap */
+static u32 io_space_blocks = 0xE1000000;
+
+u32 pci_get_io_space_block(void)
+{
+ u32 block = io_space_blocks;
+ io_space_blocks += PCI_IO_SIZE;
+
+ return block;
+}
+
static void *pci_config_address_ptr(u16 port)
{
unsigned long offset;
@@ -44,11 +56,6 @@ static struct ioport_operations pci_config_address_ops = {
.io_out = pci_config_address_out,
};
-static bool pci_config_data_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
-{
- return true;
-}
-
static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_number)
{
struct pci_device_header *dev;
@@ -67,6 +74,46 @@ static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_numbe
return dev != NULL;
}
+static bool pci_config_data_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
+{
+ unsigned long start;
+ u8 dev_num;
+
+ /*
+ * If someone accesses PCI configuration space offsets that are not
+ * aligned to 4 bytes, it uses ioports to signify that.
+ */
+ start = port - PCI_CONFIG_DATA;
+
+ dev_num = pci_config_address.device_number;
+
+ if (pci_device_exists(0, dev_num, 0)) {
+ unsigned long offset;
+
+ offset = start + (pci_config_address.register_number << 2);
+ if (offset < sizeof(struct pci_device_header)) {
+ void *p = pci_devices[dev_num];
+ u32 sz = PCI_IO_SIZE;
+
+ /*
+ * If the kernel masks the BAR it would expect to find the
+ * size of the BAR there next time it reads from it.
+ * When the kernel got the size it would write the address
+ * back.
+ */
+ if ((offset >= offsetof(struct pci_device_header, bar[0])) &&
+ (offset <= offsetof(struct pci_device_header, bar[6]))) {
+ if (*(u32 *)(p + offset))
+ memcpy(p + offset, &sz, sizeof(sz));
+ } else if (*(u32 *)(p + offset)) {
+ memcpy(p + offset, data, size);
+ }
+ }
+ }
+
+ return true;
+}
+
static bool pci_config_data_in(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
{
unsigned long start;
--
1.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] kvm tools: Fix PCI probing
2011-07-28 9:01 ` [PATCH 2/4] kvm tools: Fix PCI probing Sasha Levin
@ 2011-07-28 9:31 ` Pekka Enberg
2011-07-28 9:38 ` Cyrill Gorcunov
0 siblings, 1 reply; 12+ messages in thread
From: Pekka Enberg @ 2011-07-28 9:31 UTC (permalink / raw)
To: Sasha Levin; +Cc: kvm, mingo, asias.hejun, gorcunov, prasadjoshi124
On Thu, Jul 28, 2011 at 12:01 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> PCI BAR probing is done in four steps:
>
> 1. Read address (and flags).
> 2. Mask BAR.
> 3. Read BAR again - Now the expected result is the size of the BAR.
> 4. Mask BAR with address.
>
> So far, we have only took care of the first step. This means that the kernel
> was using address as the size, causing a PCI allocation blunder.
>
> This patch fixes the issue by passing a proper size after masking.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> tools/kvm/include/kvm/pci.h | 1 +
> tools/kvm/pci.c | 57 +++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/tools/kvm/include/kvm/pci.h b/tools/kvm/include/kvm/pci.h
> index 6ad4426..a7532e3 100644
> --- a/tools/kvm/include/kvm/pci.h
> +++ b/tools/kvm/include/kvm/pci.h
> @@ -51,5 +51,6 @@ struct pci_device_header {
>
> void pci__init(void);
> void pci__register(struct pci_device_header *dev, u8 dev_num);
> +u32 pci_get_io_space_block(void);
s/pci_get_io_space_block/pci__get_io_space_block/
>
> #endif /* KVM__PCI_H */
> diff --git a/tools/kvm/pci.c b/tools/kvm/pci.c
> index a1ad8ba..799536e3 100644
> --- a/tools/kvm/pci.c
> +++ b/tools/kvm/pci.c
> @@ -5,11 +5,23 @@
> #include <assert.h>
>
> #define PCI_MAX_DEVICES 256
> +#define PCI_IO_SIZE 0x100
>
> static struct pci_device_header *pci_devices[PCI_MAX_DEVICES];
>
> static struct pci_config_address pci_config_address;
>
> +/* This is within our PCI gap */
> +static u32 io_space_blocks = 0xE1000000;
The magic number wants to be a constant. Preferably somewhere near
where we specify the PCI gap in.
> +
> +u32 pci_get_io_space_block(void)
> +{
> + u32 block = io_space_blocks;
> + io_space_blocks += PCI_IO_SIZE;
> +
> + return block;
> +}
> +
> static void *pci_config_address_ptr(u16 port)
> {
> unsigned long offset;
> @@ -44,11 +56,6 @@ static struct ioport_operations pci_config_address_ops = {
> .io_out = pci_config_address_out,
> };
>
> -static bool pci_config_data_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
> -{
> - return true;
> -}
> -
> static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_number)
> {
> struct pci_device_header *dev;
> @@ -67,6 +74,46 @@ static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_numbe
> return dev != NULL;
> }
>
> +static bool pci_config_data_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
> +{
> + unsigned long start;
> + u8 dev_num;
> +
> + /*
> + * If someone accesses PCI configuration space offsets that are not
> + * aligned to 4 bytes, it uses ioports to signify that.
> + */
> + start = port - PCI_CONFIG_DATA;
> +
> + dev_num = pci_config_address.device_number;
> +
> + if (pci_device_exists(0, dev_num, 0)) {
> + unsigned long offset;
> +
> + offset = start + (pci_config_address.register_number << 2);
> + if (offset < sizeof(struct pci_device_header)) {
> + void *p = pci_devices[dev_num];
> + u32 sz = PCI_IO_SIZE;
> +
> + /*
> + * If the kernel masks the BAR it would expect to find the
> + * size of the BAR there next time it reads from it.
> + * When the kernel got the size it would write the address
> + * back.
> + */
> + if ((offset >= offsetof(struct pci_device_header, bar[0])) &&
> + (offset <= offsetof(struct pci_device_header, bar[6]))) {
Maybe add a helper for the bar offset checks?
> + if (*(u32 *)(p + offset))
> + memcpy(p + offset, &sz, sizeof(sz));
> + } else if (*(u32 *)(p + offset)) {
> + memcpy(p + offset, data, size);
> + }
That if-else maze is pretty scary and needs to be simplified.
> + }
> + }
> +
> + return true;
> +}
> +
> static bool pci_config_data_in(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
> {
> unsigned long start;
> --
> 1.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] kvm tools: Fix PCI probing
2011-07-28 9:31 ` Pekka Enberg
@ 2011-07-28 9:38 ` Cyrill Gorcunov
2011-07-28 9:43 ` Pekka Enberg
0 siblings, 1 reply; 12+ messages in thread
From: Cyrill Gorcunov @ 2011-07-28 9:38 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Sasha Levin, kvm, mingo, asias.hejun, prasadjoshi124
On Thu, Jul 28, 2011 at 12:31:51PM +0300, Pekka Enberg wrote:
> On Thu, Jul 28, 2011 at 12:01 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > PCI BAR probing is done in four steps:
> >
> > 1. Read address (and flags).
> > 2. Mask BAR.
> > 3. Read BAR again - Now the expected result is the size of the BAR.
> > 4. Mask BAR with address.
> >
> > So far, we have only took care of the first step. This means that the kernel
> > was using address as the size, causing a PCI allocation blunder.
> >
> > This patch fixes the issue by passing a proper size after masking.
> >
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > tools/kvm/include/kvm/pci.h | 1 +
> > tools/kvm/pci.c | 57 +++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 53 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/kvm/include/kvm/pci.h b/tools/kvm/include/kvm/pci.h
> > index 6ad4426..a7532e3 100644
> > --- a/tools/kvm/include/kvm/pci.h
> > +++ b/tools/kvm/include/kvm/pci.h
> > @@ -51,5 +51,6 @@ struct pci_device_header {
> >
> > void pci__init(void);
> > void pci__register(struct pci_device_header *dev, u8 dev_num);
> > +u32 pci_get_io_space_block(void);
>
> s/pci_get_io_space_block/pci__get_io_space_block/
>
Pekka, can we drop this idea with double underscopes? iirc perf is about
to drop them too.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] kvm tools: Fix PCI probing
2011-07-28 9:38 ` Cyrill Gorcunov
@ 2011-07-28 9:43 ` Pekka Enberg
0 siblings, 0 replies; 12+ messages in thread
From: Pekka Enberg @ 2011-07-28 9:43 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Sasha Levin, kvm, mingo, asias.hejun, prasadjoshi124
On Thu, Jul 28, 2011 at 12:38 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>> > @@ -51,5 +51,6 @@ struct pci_device_header {
>> >
>> > void pci__init(void);
>> > void pci__register(struct pci_device_header *dev, u8 dev_num);
>> > +u32 pci_get_io_space_block(void);
>>
>> s/pci_get_io_space_block/pci__get_io_space_block/
>
> Pekka, can we drop this idea with double underscopes? iirc perf is about
> to drop them too.
Ingo, is that so?
I any case, we're not doing that in *this* patch so I'd really prefer
that the inconsistency is fixed. :-)
Pekka
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback
2011-07-28 9:01 [PATCH 1/4] kvm tools: Use GSI routing Sasha Levin
2011-07-28 9:01 ` [PATCH 2/4] kvm tools: Fix PCI probing Sasha Levin
@ 2011-07-28 9:01 ` Sasha Levin
2011-07-28 9:42 ` Cyrill Gorcunov
2011-07-28 9:01 ` [PATCH 4/4] kvm tools: Implement MSI-X for virtio-rng Sasha Levin
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2011-07-28 9:01 UTC (permalink / raw)
To: penberg; +Cc: kvm, mingo, asias.hejun, gorcunov, prasadjoshi124, Sasha Levin
This makes MMIO callback similar to it's PIO counterpart by passing
a void* value provided in the registration to the callback function.
This allows to keep context within the MMIO callback function.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/include/kvm/kvm.h | 2 +-
tools/kvm/mmio.c | 8 +++++---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index eeb55b6..5f3cbbf 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -64,7 +64,7 @@ void kvm__irq_line(struct kvm *kvm, int irq, int level);
bool kvm__emulate_io(struct kvm *kvm, u16 port, void *data, int direction, int size, u32 count);
bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_write);
void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr);
-bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write));
+bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), void *ptr);
bool kvm__deregister_mmio(struct kvm *kvm, u64 phys_addr);
void kvm__pause(void);
void kvm__continue(void);
diff --git a/tools/kvm/mmio.c b/tools/kvm/mmio.c
index 64bef37..9a7f84a 100644
--- a/tools/kvm/mmio.c
+++ b/tools/kvm/mmio.c
@@ -14,7 +14,8 @@
struct mmio_mapping {
struct rb_int_node node;
- void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write);
+ void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr);
+ void *ptr;
};
static struct rb_root mmio_tree = RB_ROOT;
@@ -55,7 +56,7 @@ static const char *to_direction(u8 is_write)
return "read";
}
-bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write))
+bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), void *ptr)
{
struct mmio_mapping *mmio;
struct kvm_coalesced_mmio_zone zone;
@@ -68,6 +69,7 @@ bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, void
*mmio = (struct mmio_mapping) {
.node = RB_INT_INIT(phys_addr, phys_addr + phys_addr_len),
.kvm_mmio_callback_fn = kvm_mmio_callback_fn,
+ .ptr = ptr,
};
zone = (struct kvm_coalesced_mmio_zone) {
@@ -120,7 +122,7 @@ bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_
mmio = mmio_search(&mmio_tree, phys_addr, len);
if (mmio)
- mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write);
+ mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write, mmio->ptr);
else
fprintf(stderr, "Warning: Ignoring MMIO %s at %016llx (length %u)\n",
to_direction(is_write), phys_addr, len);
--
1.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback
2011-07-28 9:01 ` [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback Sasha Levin
@ 2011-07-28 9:42 ` Cyrill Gorcunov
2011-07-28 9:43 ` Pekka Enberg
0 siblings, 1 reply; 12+ messages in thread
From: Cyrill Gorcunov @ 2011-07-28 9:42 UTC (permalink / raw)
To: Sasha Levin; +Cc: penberg, kvm, mingo, asias.hejun, prasadjoshi124
On Thu, Jul 28, 2011 at 12:01:54PM +0300, Sasha Levin wrote:
...
>
> struct mmio_mapping {
> struct rb_int_node node;
> - void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write);
> + void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr);
> + void *ptr;
> };
I guess no need to name it *that* long, probably simple
struct mmio_mapping {
struct rb_int_node node;
void (*mmio_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr);
void *ptr;
};
...
>
> if (mmio)
> - mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write);
> + mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write, mmio->ptr);
So this would be
if (mmio)
mmio->mmio_fn(phys_addr, data, len, is_write, mmio->ptr);
no?
Cyrill
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback
2011-07-28 9:42 ` Cyrill Gorcunov
@ 2011-07-28 9:43 ` Pekka Enberg
0 siblings, 0 replies; 12+ messages in thread
From: Pekka Enberg @ 2011-07-28 9:43 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Sasha Levin, kvm, mingo, asias.hejun, prasadjoshi124
On Thu, Jul 28, 2011 at 12:42 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On Thu, Jul 28, 2011 at 12:01:54PM +0300, Sasha Levin wrote:
> ...
>>
>> struct mmio_mapping {
>> struct rb_int_node node;
>> - void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write);
>> + void (*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr);
>> + void *ptr;
>> };
>
> I guess no need to name it *that* long, probably simple
>
> struct mmio_mapping {
> struct rb_int_node node;
> void (*mmio_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr);
> void *ptr;
> };
> ...
>>
>> if (mmio)
>> - mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write);
>> + mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write, mmio->ptr);
>
> So this would be
>
> if (mmio)
> mmio->mmio_fn(phys_addr, data, len, is_write, mmio->ptr);
>
> no?
Makes sense, yes.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] kvm tools: Implement MSI-X for virtio-rng
2011-07-28 9:01 [PATCH 1/4] kvm tools: Use GSI routing Sasha Levin
2011-07-28 9:01 ` [PATCH 2/4] kvm tools: Fix PCI probing Sasha Levin
2011-07-28 9:01 ` [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback Sasha Levin
@ 2011-07-28 9:01 ` Sasha Levin
2011-07-28 9:33 ` Pekka Enberg
2011-07-28 9:27 ` [PATCH 1/4] kvm tools: Use GSI routing Pekka Enberg
2011-07-28 9:43 ` Cyrill Gorcunov
4 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2011-07-28 9:01 UTC (permalink / raw)
To: penberg; +Cc: kvm, mingo, asias.hejun, gorcunov, prasadjoshi124, Sasha Levin
This patch implements basic MSI-X support for virtio-rng.
The device uses the virtio preferred method of working with MSI-X by
creating one vector for configuration and one vector for each vq in the
device.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/include/kvm/pci.h | 16 +++++++++++++
tools/kvm/virtio/rng.c | 53 ++++++++++++++++++++++++++++++++++++++----
2 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/tools/kvm/include/kvm/pci.h b/tools/kvm/include/kvm/pci.h
index a7532e3..27fa349 100644
--- a/tools/kvm/include/kvm/pci.h
+++ b/tools/kvm/include/kvm/pci.h
@@ -24,6 +24,21 @@ struct pci_config_address {
unsigned enable_bit : 1; /* 31 */
};
+struct msix_table {
+ u32 low;
+ u32 high;
+ u32 data;
+ u32 ctrl;
+};
+
+struct msix_cap {
+ u8 cap;
+ u8 next;
+ u16 table_size;
+ u32 table_offset;
+ struct msix_table table[3 * PCI_MSIX_ENTRY_SIZE];
+};
+
struct pci_device_header {
u16 vendor_id;
u16 device_id;
@@ -47,6 +62,7 @@ struct pci_device_header {
u8 irq_pin;
u8 min_gnt;
u8 max_lat;
+ struct msix_cap msix;
};
void pci__init(void);
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index 1a7f569..6841623 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -39,6 +39,8 @@ struct rng_dev {
u8 isr;
u16 config_vector;
int fd;
+ u32 vq_vector[NUM_VIRT_QUEUES];
+ u32 msix_io_block;
/* virtio queue */
u16 queue_selector;
@@ -81,6 +83,9 @@ static bool virtio_rng_pci_io_in(struct ioport *ioport, struct kvm *kvm, u16 por
case VIRTIO_MSI_CONFIG_VECTOR:
ioport__write16(data, rdev->config_vector);
break;
+ case VIRTIO_MSI_QUEUE_VECTOR:
+ ioport__write16(data, rdev->vq_vector[rdev->queue_selector]);
+ break;
default:
ret = false;
break;
@@ -109,10 +114,10 @@ static void virtio_rng_do_io(struct kvm *kvm, void *param)
struct virt_queue *vq = job->vq;
struct rng_dev *rdev = job->rdev;
- while (virt_queue__available(vq)) {
+ while (virt_queue__available(vq))
virtio_rng_do_io_request(kvm, rdev, vq);
- virt_queue__trigger_irq(vq, rdev->pci_hdr.irq_line, &rdev->isr, kvm);
- }
+
+ kvm__irq_line(kvm, rdev->pci_hdr.irq_line, VIRTIO_IRQ_HIGH);
}
static bool virtio_rng_pci_io_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size, u32 count)
@@ -125,7 +130,6 @@ static bool virtio_rng_pci_io_out(struct ioport *ioport, struct kvm *kvm, u16 po
offset = port - rdev->base_addr;
switch (offset) {
- case VIRTIO_MSI_QUEUE_VECTOR:
case VIRTIO_PCI_GUEST_FEATURES:
break;
case VIRTIO_PCI_QUEUE_PFN: {
@@ -163,8 +167,21 @@ static bool virtio_rng_pci_io_out(struct ioport *ioport, struct kvm *kvm, u16 po
rdev->status = ioport__read8(data);
break;
case VIRTIO_MSI_CONFIG_VECTOR:
- rdev->config_vector = VIRTIO_MSI_NO_VECTOR;
+ rdev->config_vector = ioport__read16(data);
break;
+ case VIRTIO_MSI_QUEUE_VECTOR:
+ {
+ u32 gsi;
+ u32 vec;
+
+ vec = rdev->vq_vector[rdev->queue_selector] = ioport__read16(data);
+
+ gsi = irq__add_msix_route(rdev->pci_hdr.msix.table[vec].low,
+ rdev->pci_hdr.msix.table[vec].high,
+ rdev->pci_hdr.msix.table[vec].data);
+ rdev->pci_hdr.irq_line = gsi;
+ break;
+ }
default:
ret = false;
break;
@@ -185,6 +202,16 @@ static void ioevent_callback(struct kvm *kvm, void *param)
thread_pool__do_job(&job->job_id);
}
+static void callback_mmio(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr)
+{
+ struct rng_dev *rdev = ptr;
+ void *table = &rdev->pci_hdr.msix.table;
+ if (is_write)
+ memcpy(table + addr - rdev->msix_io_block, data, len);
+ else
+ memcpy(data, table + addr - rdev->msix_io_block, len);
+}
+
void virtio_rng__init(struct kvm *kvm)
{
u8 pin, line, dev, i;
@@ -196,7 +223,10 @@ void virtio_rng__init(struct kvm *kvm)
if (rdev == NULL)
return;
+ rdev->msix_io_block = pci_get_io_space_block();
+
rdev_base_addr = ioport__register(IOPORT_EMPTY, &virtio_rng_io_ops, IOPORT_SIZE, rdev);
+ kvm__register_mmio(kvm, rdev->msix_io_block, 0x100, callback_mmio, rdev);
rdev->pci_hdr = (struct pci_device_header) {
.vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET,
@@ -207,8 +237,21 @@ void virtio_rng__init(struct kvm *kvm)
.subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
.subsys_id = VIRTIO_ID_RNG,
.bar[0] = rdev_base_addr | PCI_BASE_ADDRESS_SPACE_IO,
+ .bar[1] = rdev->msix_io_block |
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ /* bar[2] is the continuation of bar[1] for 64bit addressing */
+ .bar[2] = 0,
+ .status = PCI_STATUS_CAP_LIST,
+ .capabilities = (void *)&rdev->pci_hdr.msix - (void *)&rdev->pci_hdr,
};
+ rdev->pci_hdr.msix.cap = PCI_CAP_ID_MSIX;
+ rdev->pci_hdr.msix.next = 0;
+ rdev->pci_hdr.msix.table_size = (NUM_VIRT_QUEUES + 1) | PCI_MSIX_FLAGS_ENABLE;
+ rdev->pci_hdr.msix.table_offset = 1; /* Use BAR 1 */
+
+ rdev->config_vector = 0;
rdev->base_addr = rdev_base_addr;
rdev->fd = open("/dev/urandom", O_RDONLY);
if (rdev->fd < 0)
--
1.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/4] kvm tools: Implement MSI-X for virtio-rng
2011-07-28 9:01 ` [PATCH 4/4] kvm tools: Implement MSI-X for virtio-rng Sasha Levin
@ 2011-07-28 9:33 ` Pekka Enberg
0 siblings, 0 replies; 12+ messages in thread
From: Pekka Enberg @ 2011-07-28 9:33 UTC (permalink / raw)
To: Sasha Levin; +Cc: kvm, mingo, asias.hejun, gorcunov, prasadjoshi124
On Thu, Jul 28, 2011 at 12:01 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> @@ -163,8 +167,21 @@ static bool virtio_rng_pci_io_out(struct ioport *ioport, struct kvm *kvm, u16 po
> rdev->status = ioport__read8(data);
> break;
> case VIRTIO_MSI_CONFIG_VECTOR:
> - rdev->config_vector = VIRTIO_MSI_NO_VECTOR;
> + rdev->config_vector = ioport__read16(data);
> break;
> + case VIRTIO_MSI_QUEUE_VECTOR:
> + {
Nit: the curly brace should be on the same line as 'case' keyword.
> + u32 gsi;
> + u32 vec;
> +
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] kvm tools: Use GSI routing
2011-07-28 9:01 [PATCH 1/4] kvm tools: Use GSI routing Sasha Levin
` (2 preceding siblings ...)
2011-07-28 9:01 ` [PATCH 4/4] kvm tools: Implement MSI-X for virtio-rng Sasha Levin
@ 2011-07-28 9:27 ` Pekka Enberg
2011-07-28 9:43 ` Cyrill Gorcunov
4 siblings, 0 replies; 12+ messages in thread
From: Pekka Enberg @ 2011-07-28 9:27 UTC (permalink / raw)
To: Sasha Levin; +Cc: kvm, mingo, asias.hejun, gorcunov, prasadjoshi124
Hi Sasha,
On Thu, Jul 28, 2011 at 12:01 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> Map GSIs manually when starting the guest.
> This will allow us mapping new GSIs for MSIX in the future.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> tools/kvm/builtin-run.c | 3 ++
> tools/kvm/include/kvm/irq.h | 3 ++
> tools/kvm/irq.c | 75 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+), 0 deletions(-)
>
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 941129c..f9c3e7e 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -35,6 +35,7 @@
> #include <kvm/vnc.h>
> #include <kvm/sdl.h>
> #include <kvm/framebuffer.h>
> +#include <kvm/irq.h>
>
> /* header files for gitish interface */
> #include <kvm/builtin-run.h>
> @@ -573,6 +574,8 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>
> kvm = kvm__init(kvm_dev, ram_size, guest_name);
>
> + irq__init();
> +
> kvm->single_step = single_step;
>
> ioeventfd__init();
> diff --git a/tools/kvm/include/kvm/irq.h b/tools/kvm/include/kvm/irq.h
> index 7a75a0c..2f00b18 100644
> --- a/tools/kvm/include/kvm/irq.h
> +++ b/tools/kvm/include/kvm/irq.h
> @@ -21,4 +21,7 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line);
>
> struct rb_node *irq__get_pci_tree(void);
>
> +void irq__init(void);
> +u32 irq__add_msix_route(u32 low, u32 high, u32 data);
> +
> #endif
> diff --git a/tools/kvm/irq.c b/tools/kvm/irq.c
> index 15f4702..fc5d800 100644
> --- a/tools/kvm/irq.c
> +++ b/tools/kvm/irq.c
> @@ -1,16 +1,47 @@
> #include "kvm/irq.h"
> +#include "kvm/kvm.h"
> +#include "kvm/util.h"
>
> #include <linux/types.h>
> #include <linux/rbtree.h>
> #include <linux/list.h>
> +#include <linux/kvm.h>
> +#include <sys/ioctl.h>
>
> #include <stddef.h>
> #include <stdlib.h>
>
> +#define IRQ_MAX_GSI 64
> +#define IRQCHIP_MASTER 0
> +#define IRQCHIP_SLAVE 1
> +#define IRQCHIP_IOAPIC 2
> +
> static u8 next_line = 3;
> static u8 next_dev = 1;
> static struct rb_root pci_tree = RB_ROOT;
>
> +/* First 24 GSIs are routed between IRQCHIPs and IOAPICs */
> +static u32 gsi = 24;
> +
> +extern struct kvm *kvm;
I'd prefer you actually pass this as an argument to irq__init() and
irq__add_msix_route().
> +
> +struct kvm_irq_routing *irq_routing;
> +
> +static int irq__add_routing(u32 gsi, u32 type, u32 irqchip, u32 pin)
> +{
> + if (gsi >= IRQ_MAX_GSI)
> + return -ENOSPC;
EINVAL is more appropriate, I think.
> +
> + irq_routing->entries[irq_routing->nr++] = (struct kvm_irq_routing_entry) {
> + .gsi = gsi,
> + .type = type,
> + .u.irqchip.irqchip = irqchip,
> + .u.irqchip.pin = pin,
Please indent the assignments.
> + };
> +
> + return 0;
> +}
> +
> static struct pci_dev *search(struct rb_root *root, u32 id)
> {
> struct rb_node *node = root->rb_node;
> @@ -106,6 +137,50 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
> return -1;
> }
>
> +void irq__init(void)
> +{
> + int i, r;
> +
> + irq_routing = malloc(sizeof(struct kvm_irq_routing) +
> + IRQ_MAX_GSI * sizeof(struct kvm_irq_routing_entry));
We usually die() on out-of-memory. Maybe it doesn't make sense here. Dunno.
> +
> + /* Hook first 8 GSIs to master IRQCHIP */
> + for (i = 0; i < 8; i++)
> + irq__add_routing(i, KVM_IRQ_ROUTING_IRQCHIP, IRQCHIP_MASTER, i);
> +
> + /* Hook next 8 GSIs to slave IRQCHIP */
> + for (i = 8; i < 16; i++)
> + irq__add_routing(i, KVM_IRQ_ROUTING_IRQCHIP, IRQCHIP_SLAVE, 8-i);
> +
> + /* Last but not least, IOAPIC */
> + for (i = 0; i < 24; i++) {
> + if (i == 0)
> + irq__add_routing(i, KVM_IRQ_ROUTING_IRQCHIP, IRQCHIP_IOAPIC, 2);
> + else
> + irq__add_routing(i, KVM_IRQ_ROUTING_IRQCHIP, IRQCHIP_IOAPIC, i);
> + }
> +
> + r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
> + if (r)
> + die("Failed setting GSI routes");
> +}
> +
> +u32 irq__add_msix_route(u32 low, u32 high, u32 data)
> +{
> + irq_routing->entries[irq_routing->nr++] = (struct kvm_irq_routing_entry) {
> + .gsi = gsi,
> + .type = KVM_IRQ_ROUTING_MSI,
> + .u.msi.address_lo = low,
> + .u.msi.address_hi = high,
> + .u.msi.data = data,
Please indent the assignments.
> + };
> +
> + if (ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing))
> + return -1;
How will that work when we have 'u32' as the return value type?
> +
> + return gsi++;
> +}
> +
> struct rb_node *irq__get_pci_tree(void)
> {
> return rb_first(&pci_tree);
> --
> 1.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] kvm tools: Use GSI routing
2011-07-28 9:01 [PATCH 1/4] kvm tools: Use GSI routing Sasha Levin
` (3 preceding siblings ...)
2011-07-28 9:27 ` [PATCH 1/4] kvm tools: Use GSI routing Pekka Enberg
@ 2011-07-28 9:43 ` Cyrill Gorcunov
4 siblings, 0 replies; 12+ messages in thread
From: Cyrill Gorcunov @ 2011-07-28 9:43 UTC (permalink / raw)
To: Sasha Levin; +Cc: penberg, kvm, mingo, asias.hejun, prasadjoshi124
On Thu, Jul 28, 2011 at 12:01:52PM +0300, Sasha Levin wrote:
> Map GSIs manually when starting the guest.
> This will allow us mapping new GSIs for MSIX in the future.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
Other than a few nits the series looks good to me, thanks Sasha!
^ permalink raw reply [flat|nested] 12+ messages in thread