* [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-04 10:42 ` Michael S. Tsirkin
2011-12-04 13:12 ` Avi Kivity
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder Jan Kiszka
` (14 subsequent siblings)
15 siblings, 2 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Rename msix_supported to msi_supported and control MSI and MSI-X
activation this way. That was likely to original intention for this
flag, but MSI support came after MSI-X.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/msi.c | 8 ++++++++
hw/msi.h | 2 ++
hw/msix.c | 9 ++++-----
hw/msix.h | 2 --
hw/pc.c | 4 ++--
5 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/hw/msi.c b/hw/msi.c
index f214fcf..5d6ceb6 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -36,6 +36,9 @@
#define PCI_MSI_VECTORS_MAX 32
+/* Flag for interrupt controller to declare MSI/MSI-X support */
+bool msi_supported;
+
/* If we get rid of cap allocator, we won't need this. */
static inline uint8_t msi_cap_sizeof(uint16_t flags)
{
@@ -116,6 +119,11 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
uint16_t flags;
uint8_t cap_size;
int config_offset;
+
+ if (!msi_supported) {
+ return -ENOTSUP;
+ }
+
MSI_DEV_PRINTF(dev,
"init offset: 0x%"PRIx8" vector: %"PRId8
" 64bit %d mask %d\n",
diff --git a/hw/msi.h b/hw/msi.h
index 5766018..3040bb0 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -24,6 +24,8 @@
#include "qemu-common.h"
#include "pci.h"
+extern bool msi_supported;
+
bool msi_enabled(const PCIDevice *dev);
int msi_init(struct PCIDevice *dev, uint8_t offset,
unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask);
diff --git a/hw/msix.c b/hw/msix.c
index b15bafc..8850fbd 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -12,6 +12,7 @@
*/
#include "hw.h"
+#include "msi.h"
#include "msix.h"
#include "pci.h"
#include "range.h"
@@ -32,9 +33,6 @@
#define MSIX_MAX_ENTRIES 32
-/* Flag for interrupt controller to declare MSI-X support */
-int msix_supported;
-
/* Add MSI-X capability to the config space for the device. */
/* Given a bar and its size, add MSI-X table on top of it
* and fill MSI-X capability in the config space.
@@ -212,10 +210,11 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
unsigned bar_nr, unsigned bar_size)
{
int ret;
+
/* Nothing to do if MSI is not supported by interrupt controller */
- if (!msix_supported)
+ if (!msi_supported) {
return -ENOTSUP;
-
+ }
if (nentries > MSIX_MAX_ENTRIES)
return -EINVAL;
diff --git a/hw/msix.h b/hw/msix.h
index 7e04336..5aba22b 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -29,6 +29,4 @@ void msix_notify(PCIDevice *dev, unsigned vector);
void msix_reset(PCIDevice *dev);
-extern int msix_supported;
-
#endif
diff --git a/hw/pc.c b/hw/pc.c
index 9328ee5..5225d5b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -36,7 +36,7 @@
#include "elf.h"
#include "multiboot.h"
#include "mc146818rtc.h"
-#include "msix.h"
+#include "msi.h"
#include "sysbus.h"
#include "sysemu.h"
#include "blockdev.h"
@@ -896,7 +896,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
apic_mapped = 1;
}
- msix_supported = 1;
+ msi_supported = true;
return dev;
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported Jan Kiszka
@ 2011-12-04 10:42 ` Michael S. Tsirkin
2011-12-04 10:42 ` Jan Kiszka
2011-12-04 13:12 ` Avi Kivity
1 sibling, 1 reply; 54+ messages in thread
From: Michael S. Tsirkin @ 2011-12-04 10:42 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Marcelo Tosatti, qemu-devel, Blue Swirl,
Avi Kivity
On Sat, Dec 03, 2011 at 12:17:26PM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Rename msix_supported to msi_supported and control MSI and MSI-X
> activation this way. That was likely to original intention for this
> flag, but MSI support came after MSI-X.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
This patch should go into qemu.git, right?
> ---
> hw/msi.c | 8 ++++++++
> hw/msi.h | 2 ++
> hw/msix.c | 9 ++++-----
> hw/msix.h | 2 --
> hw/pc.c | 4 ++--
> 5 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/hw/msi.c b/hw/msi.c
> index f214fcf..5d6ceb6 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -36,6 +36,9 @@
>
> #define PCI_MSI_VECTORS_MAX 32
>
> +/* Flag for interrupt controller to declare MSI/MSI-X support */
> +bool msi_supported;
> +
> /* If we get rid of cap allocator, we won't need this. */
> static inline uint8_t msi_cap_sizeof(uint16_t flags)
> {
> @@ -116,6 +119,11 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
> uint16_t flags;
> uint8_t cap_size;
> int config_offset;
> +
> + if (!msi_supported) {
> + return -ENOTSUP;
> + }
> +
> MSI_DEV_PRINTF(dev,
> "init offset: 0x%"PRIx8" vector: %"PRId8
> " 64bit %d mask %d\n",
> diff --git a/hw/msi.h b/hw/msi.h
> index 5766018..3040bb0 100644
> --- a/hw/msi.h
> +++ b/hw/msi.h
> @@ -24,6 +24,8 @@
> #include "qemu-common.h"
> #include "pci.h"
>
> +extern bool msi_supported;
> +
> bool msi_enabled(const PCIDevice *dev);
> int msi_init(struct PCIDevice *dev, uint8_t offset,
> unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask);
> diff --git a/hw/msix.c b/hw/msix.c
> index b15bafc..8850fbd 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -12,6 +12,7 @@
> */
>
> #include "hw.h"
> +#include "msi.h"
> #include "msix.h"
> #include "pci.h"
> #include "range.h"
> @@ -32,9 +33,6 @@
> #define MSIX_MAX_ENTRIES 32
>
>
> -/* Flag for interrupt controller to declare MSI-X support */
> -int msix_supported;
> -
> /* Add MSI-X capability to the config space for the device. */
> /* Given a bar and its size, add MSI-X table on top of it
> * and fill MSI-X capability in the config space.
> @@ -212,10 +210,11 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
> unsigned bar_nr, unsigned bar_size)
> {
> int ret;
> +
> /* Nothing to do if MSI is not supported by interrupt controller */
> - if (!msix_supported)
> + if (!msi_supported) {
> return -ENOTSUP;
> -
> + }
> if (nentries > MSIX_MAX_ENTRIES)
> return -EINVAL;
>
> diff --git a/hw/msix.h b/hw/msix.h
> index 7e04336..5aba22b 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -29,6 +29,4 @@ void msix_notify(PCIDevice *dev, unsigned vector);
>
> void msix_reset(PCIDevice *dev);
>
> -extern int msix_supported;
> -
> #endif
> diff --git a/hw/pc.c b/hw/pc.c
> index 9328ee5..5225d5b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -36,7 +36,7 @@
> #include "elf.h"
> #include "multiboot.h"
> #include "mc146818rtc.h"
> -#include "msix.h"
> +#include "msi.h"
> #include "sysbus.h"
> #include "sysemu.h"
> #include "blockdev.h"
> @@ -896,7 +896,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
> apic_mapped = 1;
> }
>
> - msix_supported = 1;
> + msi_supported = true;
>
> return dev;
> }
> --
> 1.7.3.4
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported
2011-12-04 10:42 ` Michael S. Tsirkin
@ 2011-12-04 10:42 ` Jan Kiszka
0 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 10:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, kvm, Marcelo Tosatti, qemu-devel, Blue Swirl,
Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
On 2011-12-04 11:42, Michael S. Tsirkin wrote:
> On Sat, Dec 03, 2011 at 12:17:26PM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Rename msix_supported to msi_supported and control MSI and MSI-X
>> activation this way. That was likely to original intention for this
>> flag, but MSI support came after MSI-X.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> This patch should go into qemu.git, right?
Right. It was just that this series depends on it. Feel free to pick it
up earlier.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported Jan Kiszka
2011-12-04 10:42 ` Michael S. Tsirkin
@ 2011-12-04 13:12 ` Avi Kivity
2011-12-04 13:16 ` Jan Kiszka
1 sibling, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:12 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Rename msix_supported to msi_supported and control MSI and MSI-X
> activation this way. That was likely to original intention for this
> flag, but MSI support came after MSI-X.
'and' is a dangerous word in a changelog entry.
>
> +
> + if (!msi_supported) {
> + return -ENOTSUP;
> + }
> +
>
This changes behaviour. qemu 1.0 -M pc-1.0 and qemu-1.1 -M pc-1.0 will
be different after this, no?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported
2011-12-04 13:12 ` Avi Kivity
@ 2011-12-04 13:16 ` Jan Kiszka
2011-12-04 13:26 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 13:16 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
On 2011-12-04 14:12, Avi Kivity wrote:
> On 12/03/2011 01:17 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Rename msix_supported to msi_supported and control MSI and MSI-X
>> activation this way. That was likely to original intention for this
>> flag, but MSI support came after MSI-X.
>
> 'and' is a dangerous word in a changelog entry.
This patch hardly qualifies for two IMHO.
>
>>
>> +
>> + if (!msi_supported) {
>> + return -ENOTSUP;
>> + }
>> +
>>
>
> This changes behaviour. qemu 1.0 -M pc-1.0 and qemu-1.1 -M pc-1.0 will
> be different after this, no?
>
Only isapc had msix_supported = 0, and I doubt we got there (msi_init)
for that machine. Or am I missing something?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported
2011-12-04 13:16 ` Jan Kiszka
@ 2011-12-04 13:26 ` Avi Kivity
0 siblings, 0 replies; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:26 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 03:16 PM, Jan Kiszka wrote:
> On 2011-12-04 14:12, Avi Kivity wrote:
> > On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Rename msix_supported to msi_supported and control MSI and MSI-X
> >> activation this way. That was likely to original intention for this
> >> flag, but MSI support came after MSI-X.
> >
> > 'and' is a dangerous word in a changelog entry.
>
> This patch hardly qualifies for two IMHO.
If we don't have to change it, no.
>
> >
> >>
> >> +
> >> + if (!msi_supported) {
> >> + return -ENOTSUP;
> >> + }
> >> +
> >>
> >
> > This changes behaviour. qemu 1.0 -M pc-1.0 and qemu-1.1 -M pc-1.0 will
> > be different after this, no?
> >
>
> Only isapc had msix_supported = 0, and I doubt we got there (msi_init)
> for that machine. Or am I missing something?
>
Ah, I thought it was a user-settable property, but it isn't.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 19:00 ` Andreas Färber
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 03/16] apic: Stop timer on reset Jan Kiszka
` (13 subsequent siblings)
15 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
More KVM-specific devices will come, so let's start with moving the
kvmclock into a dedicated folder.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.target | 4 ++--
configure | 1 +
hw/{kvmclock.c => kvm/clock.c} | 10 +++++-----
hw/{kvmclock.h => kvm/clock.h} | 0
hw/pc_piix.c | 2 +-
5 files changed, 9 insertions(+), 8 deletions(-)
rename hw/{kvmclock.c => kvm/clock.c} (96%)
rename hw/{kvmclock.h => kvm/clock.h} (100%)
diff --git a/Makefile.target b/Makefile.target
index 1e90df7..3a9e95d 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -231,7 +231,7 @@ obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += debugcon.o multiboot.o
obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvmclock.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
# shared objects
@@ -421,7 +421,7 @@ qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
- rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
+ rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o kvm/*.o
rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
ifdef CONFIG_TRACE_SYSTEMTAP
rm -f *.stp
diff --git a/configure b/configure
index 4f87e0a..d768e44 100755
--- a/configure
+++ b/configure
@@ -3220,6 +3220,7 @@ mkdir -p $target_dir/fpu
mkdir -p $target_dir/tcg
mkdir -p $target_dir/ide
mkdir -p $target_dir/9pfs
+mkdir -p $target_dir/kvm
if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o "$target" = "arm-bsd-user" -o "$target" = "armeb-bsd-user" ; then
mkdir -p $target_dir/nwfpe
fi
diff --git a/hw/kvmclock.c b/hw/kvm/clock.c
similarity index 96%
rename from hw/kvmclock.c
rename to hw/kvm/clock.c
index 5388bc4..aa37c5d 100644
--- a/hw/kvmclock.c
+++ b/hw/kvm/clock.c
@@ -11,11 +11,11 @@
*
*/
-#include "qemu-common.h"
-#include "sysemu.h"
-#include "sysbus.h"
-#include "kvm.h"
-#include "kvmclock.h"
+#include <qemu-common.h>
+#include <sysemu.h>
+#include <kvm.h>
+#include <hw/sysbus.h>
+#include <hw/kvm/clock.h>
#include <linux/kvm.h>
#include <linux/kvm_para.h>
diff --git a/hw/kvmclock.h b/hw/kvm/clock.h
similarity index 100%
rename from hw/kvmclock.h
rename to hw/kvm/clock.h
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index c89042f..22997b0 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -34,7 +34,7 @@
#include "boards.h"
#include "ide.h"
#include "kvm.h"
-#include "kvmclock.h"
+#include "kvm/clock.h"
#include "sysemu.h"
#include "sysbus.h"
#include "arch_init.h"
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder Jan Kiszka
@ 2011-12-03 19:00 ` Andreas Färber
2011-12-03 22:33 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Andreas Färber @ 2011-12-03 19:00 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl, Avi Kivity
Am 03.12.2011 12:17, schrieb Jan Kiszka:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> More KVM-specific devices will come, so let's start with moving the
> kvmclock into a dedicated folder.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> diff --git a/Makefile.target b/Makefile.target
> index 1e90df7..3a9e95d 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -231,7 +231,7 @@ obj-i386-y += vmport.o
> obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
> obj-i386-y += debugcon.o multiboot.o
> obj-i386-y += pc_piix.o
> -obj-i386-$(CONFIG_KVM) += kvmclock.o
> +obj-i386-$(CONFIG_KVM) += kvm/clock.o
> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>
> # shared objects
> diff --git a/hw/kvmclock.c b/hw/kvm/clock.c
> similarity index 96%
> rename from hw/kvmclock.c
> rename to hw/kvm/clock.c
> index 5388bc4..aa37c5d 100644
> --- a/hw/kvmclock.c
> +++ b/hw/kvm/clock.c
> @@ -11,11 +11,11 @@
> *
> */
>
> -#include "qemu-common.h"
> -#include "sysemu.h"
> -#include "sysbus.h"
> -#include "kvm.h"
> -#include "kvmclock.h"
> +#include <qemu-common.h>
> +#include <sysemu.h>
> +#include <kvm.h>
> +#include <hw/sysbus.h>
> +#include <hw/kvm/clock.h>
>
> #include <linux/kvm.h>
> #include <linux/kvm_para.h>
Please don't start using system includes for everything. Rather extend
QEMU_CFLAGS to contain the right user include path(s).
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder
2011-12-03 19:00 ` Andreas Färber
@ 2011-12-03 22:33 ` Jan Kiszka
2011-12-04 10:43 ` Avi Kivity
2011-12-05 10:43 ` Andreas Färber
0 siblings, 2 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 22:33 UTC (permalink / raw)
To: Andreas Färber
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]
On 2011-12-03 20:00, Andreas Färber wrote:
> Am 03.12.2011 12:17, schrieb Jan Kiszka:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> More KVM-specific devices will come, so let's start with moving the
>> kvmclock into a dedicated folder.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>
>> diff --git a/Makefile.target b/Makefile.target
>> index 1e90df7..3a9e95d 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -231,7 +231,7 @@ obj-i386-y += vmport.o
>> obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>> obj-i386-y += debugcon.o multiboot.o
>> obj-i386-y += pc_piix.o
>> -obj-i386-$(CONFIG_KVM) += kvmclock.o
>> +obj-i386-$(CONFIG_KVM) += kvm/clock.o
>> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>
>> # shared objects
>
>> diff --git a/hw/kvmclock.c b/hw/kvm/clock.c
>> similarity index 96%
>> rename from hw/kvmclock.c
>> rename to hw/kvm/clock.c
>> index 5388bc4..aa37c5d 100644
>> --- a/hw/kvmclock.c
>> +++ b/hw/kvm/clock.c
>> @@ -11,11 +11,11 @@
>> *
>> */
>>
>> -#include "qemu-common.h"
>> -#include "sysemu.h"
>> -#include "sysbus.h"
>> -#include "kvm.h"
>> -#include "kvmclock.h"
>> +#include <qemu-common.h>
>> +#include <sysemu.h>
>> +#include <kvm.h>
>> +#include <hw/sysbus.h>
>> +#include <hw/kvm/clock.h>
>>
>> #include <linux/kvm.h>
>> #include <linux/kvm_para.h>
>
> Please don't start using system includes for everything. Rather extend
> QEMU_CFLAGS to contain the right user include path(s).
No problem - and no need to tweak any CFLAGS ("" only adds . to the
header search paths).
Do we have a convention that every include in <> is considered system
header? Should probably be documented then (and code should be converted
gradually).
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder
2011-12-03 22:33 ` Jan Kiszka
@ 2011-12-04 10:43 ` Avi Kivity
2011-12-04 10:46 ` Jan Kiszka
2011-12-05 10:43 ` Andreas Färber
1 sibling, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 10:43 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl, Andreas Färber
On 12/04/2011 12:33 AM, Jan Kiszka wrote:
> Do we have a convention that every include in <> is considered system
> header? Should probably be documented then (and code should be converted
> gradually).
It's documented in "The C Programming Language", by K&R.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder
2011-12-04 10:43 ` Avi Kivity
@ 2011-12-04 10:46 ` Jan Kiszka
0 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 10:46 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 584 bytes --]
On 2011-12-04 11:43, Avi Kivity wrote:
> On 12/04/2011 12:33 AM, Jan Kiszka wrote:
>> Do we have a convention that every include in <> is considered system
>> header? Should probably be documented then (and code should be converted
>> gradually).
>
> It's documented in "The C Programming Language", by K&R.
It's just a convention, nothing more. If you consider certain parts of
QEMU's API as system (e.g. the parts that may once make our modular
API), it makes some sense to use <> for. Right now this happens for some
parts of the hw API. But inconsistently.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder
2011-12-03 22:33 ` Jan Kiszka
2011-12-04 10:43 ` Avi Kivity
@ 2011-12-05 10:43 ` Andreas Färber
1 sibling, 0 replies; 54+ messages in thread
From: Andreas Färber @ 2011-12-05 10:43 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl, Avi Kivity
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Am 03.12.2011 23:33, schrieb Jan Kiszka:
> On 2011-12-03 20:00, Andreas Färber wrote:
>> Am 03.12.2011 12:17, schrieb Jan Kiszka:
>>> diff --git a/hw/kvmclock.c b/hw/kvm/clock.c similarity index
>>> 96% rename from hw/kvmclock.c rename to hw/kvm/clock.c index
>>> 5388bc4..aa37c5d 100644 --- a/hw/kvmclock.c +++
>>> b/hw/kvm/clock.c @@ -11,11 +11,11 @@ * */
>>>
>>> -#include "qemu-common.h" -#include "sysemu.h" -#include
>>> "sysbus.h" -#include "kvm.h" -#include "kvmclock.h" +#include
>>> <qemu-common.h> +#include <sysemu.h> +#include <kvm.h>
>>> +#include <hw/sysbus.h> +#include <hw/kvm/clock.h>
>>>
>>> #include <linux/kvm.h> #include <linux/kvm_para.h>
>>
>> Please don't start using system includes for everything. Rather
>> extend QEMU_CFLAGS to contain the right user include path(s).
>
> No problem - and no need to tweak any CFLAGS
Right, I had recursion into kvm/ in mind - would've required -I ../..
to be added to CFLAGS.
> ("" only adds . to the header search paths).
By default that is. -iquote can add further paths. (Unfortunately
didn't solve the Cocoa Block.h vs. block.h problem since Objective-C
frameworks use quotes, too.)
> Do we have a convention that every include in <> is considered
> system header? Should probably be documented then (and code should
> be converted gradually).
The convention I perceived was that everything QEMU was in quotes
whereas POSIX, Linux, zlib, glib, etc. were in angle brackets. Didn't
check for documentation.
Andreas
- --
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)
iQIcBAEBAgAGBQJO3KA6AAoJEPou0S0+fgE/izQP/1q0Oje72FdXyUyVxPZw2Ypi
zp+2TFYJ3FJUrTLkkDBjmsaMT0sdIoI/wXxDTrrif9QI1gfRhNlxw9qES+En4xDG
3ClCl6UMNrcq35WrejIvPOXQMvVH6tTnliHBKmG6TSsQXPEFLS/BbWA1Y3gV7nZ4
KXmMHdNqVzmo66AU0FGQPSZyE/u+w8PKnfOIea961tMFtYodny69lzuoBWIaC/oT
8neCRT6U4BVX6hEy6QgY1651IM0KUOUC0fbBwFMwiy+NeL5KgB+GWsrnVq+U0hpM
gDtE09L1IKzuppMLlsx1DmxAZYHX12ZlW5W3np13+qDOkFx+4JqT3AU1MGBDhVQ+
ylbYXAINpcXsV8hTyCv1xoWlCJTUreD5+vVgAe5IN3jJUuXttR867YZHS6w0Xkh2
saTYRdkaywNpb9Jm/8RdP0Nepjq2YKdjP99/Da5/GOlVBOqASycKmtAyKQKerhAx
2n+Os8Ekji9fLM7S1FFWe2i/v/bUiVKb9TPRw98tDaDd9V0RW2AkBrJcL2BlFBC4
nqM57ndpv3phGLbVoin2yo32P6iTqL/bS7iyJap+IeklSzxSyW0bBcJyT0oIZMQ2
TdeZNSS2aF9+SmIp91aNRIWhXDAZGggls5AvrS3FTbyzY0jb4HXLIYVGyLCdzfar
uHBpp0n3XZsqieTYP+f0
=zA/a
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 54+ messages in thread
* [Qemu-devel] [RFC][PATCH 03/16] apic: Stop timer on reset
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 01/16] msi: Generalize msix_supported to msi_supported Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 02/16] kvm: Move kvmclock into hw/kvm folder Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 04/16] apic: Factor out core for KVM reuse Jan Kiszka
` (12 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
All LVTs are masked on reset, so the timer becomes ineffective. Letting
it tick nevertheless is harmless, but will at least create a spurious
trace event.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/apic.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index 8289eef..2644a82 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -528,6 +528,8 @@ void apic_init_reset(DeviceState *d)
s->initial_count_load_time = 0;
s->next_time = 0;
s->wait_for_sipi = 1;
+
+ qemu_del_timer(s->timer);
}
static void apic_startup(APICState *s, int vector_num)
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 04/16] apic: Factor out core for KVM reuse
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (2 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 03/16] apic: Stop timer on reset Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 05/16] apic: Open-code timer save/restore Jan Kiszka
` (11 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
The KVM in-kernel APIC model will reuse parts of the user space model,
namely the vmstate, reset handling, IRQ coalescing tracker, some init
steps and the base and tpr set/get routines. For the latter, we also
prepare set callbacks as KVM will override those.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.target | 2 +-
hw/apic.c | 260 +++-------------------------------------------------
hw/apic_common.c | 215 +++++++++++++++++++++++++++++++++++++++++++
hw/apic_internal.h | 108 ++++++++++++++++++++++
trace-events | 2 +-
5 files changed, 339 insertions(+), 248 deletions(-)
create mode 100644 hw/apic_common.c
create mode 100644 hw/apic_internal.h
diff --git a/Makefile.target b/Makefile.target
index 3a9e95d..7bb6b13 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -226,7 +226,7 @@ obj-$(CONFIG_IVSHMEM) += ivshmem.o
# Hardware support
obj-i386-y += vga.o
obj-i386-y += mc146818rtc.o pc.o
-obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o sga.o apic_common.o apic.o ioapic.o piix_pci.o
obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += debugcon.o multiboot.o
diff --git a/hw/apic.c b/hw/apic.c
index 2644a82..27b18d6 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -16,53 +16,13 @@
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>
*/
-#include "hw.h"
+#include "apic_internal.h"
#include "apic.h"
#include "ioapic.h"
-#include "qemu-timer.h"
#include "host-utils.h"
-#include "sysbus.h"
#include "trace.h"
#include "pc.h"
-/* APIC Local Vector Table */
-#define APIC_LVT_TIMER 0
-#define APIC_LVT_THERMAL 1
-#define APIC_LVT_PERFORM 2
-#define APIC_LVT_LINT0 3
-#define APIC_LVT_LINT1 4
-#define APIC_LVT_ERROR 5
-#define APIC_LVT_NB 6
-
-/* APIC delivery modes */
-#define APIC_DM_FIXED 0
-#define APIC_DM_LOWPRI 1
-#define APIC_DM_SMI 2
-#define APIC_DM_NMI 4
-#define APIC_DM_INIT 5
-#define APIC_DM_SIPI 6
-#define APIC_DM_EXTINT 7
-
-/* APIC destination mode */
-#define APIC_DESTMODE_FLAT 0xf
-#define APIC_DESTMODE_CLUSTER 1
-
-#define APIC_TRIGGER_EDGE 0
-#define APIC_TRIGGER_LEVEL 1
-
-#define APIC_LVT_TIMER_PERIODIC (1<<17)
-#define APIC_LVT_MASKED (1<<16)
-#define APIC_LVT_LEVEL_TRIGGER (1<<15)
-#define APIC_LVT_REMOTE_IRR (1<<14)
-#define APIC_INPUT_POLARITY (1<<13)
-#define APIC_SEND_PENDING (1<<12)
-
-#define ESR_ILLEGAL_ADDRESS (1 << 7)
-
-#define APIC_SV_DIRECTED_IO (1<<12)
-#define APIC_SV_ENABLE (1<<8)
-
-#define MAX_APICS 255
#define MAX_APIC_WORDS 8
/* Intel APIC constants: from include/asm/msidef.h */
@@ -75,40 +35,7 @@
#define MSI_ADDR_DEST_ID_SHIFT 12
#define MSI_ADDR_DEST_ID_MASK 0x00ffff0
-#define MSI_ADDR_SIZE 0x100000
-
-typedef struct APICState APICState;
-
-struct APICState {
- SysBusDevice busdev;
- MemoryRegion io_memory;
- void *cpu_env;
- uint32_t apicbase;
- uint8_t id;
- uint8_t arb_id;
- uint8_t tpr;
- uint32_t spurious_vec;
- uint8_t log_dest;
- uint8_t dest_mode;
- uint32_t isr[8]; /* in service register */
- uint32_t tmr[8]; /* trigger mode register */
- uint32_t irr[8]; /* interrupt request register */
- uint32_t lvt[APIC_LVT_NB];
- uint32_t esr; /* error register */
- uint32_t icr[2];
-
- uint32_t divide_conf;
- int count_shift;
- uint32_t initial_count;
- int64_t initial_count_load_time, next_time;
- uint32_t idx;
- QEMUTimer *timer;
- int sipi_vector;
- int wait_for_sipi;
-};
-
static APICState *local_apics[MAX_APICS + 1];
-static int apic_irq_delivered;
static void apic_set_irq(APICState *s, int vector_num, int trigger_mode);
static void apic_update_irq(APICState *s);
@@ -293,14 +220,8 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
}
-void cpu_set_apic_base(DeviceState *d, uint64_t val)
+static void apic_set_base(APICState *s, uint64_t val)
{
- APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
-
- trace_cpu_set_apic_base(val);
-
- if (!s)
- return;
s->apicbase = (val & 0xfffff000) |
(s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
/* if disabled, cannot be enabled again */
@@ -311,32 +232,12 @@ void cpu_set_apic_base(DeviceState *d, uint64_t val)
}
}
-uint64_t cpu_get_apic_base(DeviceState *d)
-{
- APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
-
- trace_cpu_get_apic_base(s ? (uint64_t)s->apicbase: 0);
-
- return s ? s->apicbase : 0;
-}
-
-void cpu_set_apic_tpr(DeviceState *d, uint8_t val)
+static void apic_set_tpr(APICState *s, uint8_t val)
{
- APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
-
- if (!s)
- return;
s->tpr = (val & 0x0f) << 4;
apic_update_irq(s);
}
-uint8_t cpu_get_apic_tpr(DeviceState *d)
-{
- APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
-
- return s ? s->tpr >> 4 : 0;
-}
-
/* return -1 if no bit is set */
static int get_highest_priority_int(uint32_t *tab)
{
@@ -406,25 +307,9 @@ static void apic_update_irq(APICState *s)
}
}
-void apic_reset_irq_delivered(void)
-{
- trace_apic_reset_irq_delivered(apic_irq_delivered);
-
- apic_irq_delivered = 0;
-}
-
-int apic_get_irq_delivered(void)
-{
- trace_apic_get_irq_delivered(apic_irq_delivered);
-
- return apic_irq_delivered;
-}
-
static void apic_set_irq(APICState *s, int vector_num, int trigger_mode)
{
- apic_irq_delivered += !get_bit(s->irr, vector_num);
-
- trace_apic_set_irq(apic_irq_delivered);
+ apic_set_irq_delivered(!get_bit(s->irr, vector_num));
set_bit(s->irr, vector_num);
if (trigger_mode)
@@ -503,35 +388,6 @@ static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
}
}
-void apic_init_reset(DeviceState *d)
-{
- APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
- int i;
-
- if (!s)
- return;
-
- s->tpr = 0;
- s->spurious_vec = 0xff;
- s->log_dest = 0;
- s->dest_mode = 0xf;
- memset(s->isr, 0, sizeof(s->isr));
- memset(s->tmr, 0, sizeof(s->tmr));
- memset(s->irr, 0, sizeof(s->irr));
- for(i = 0; i < APIC_LVT_NB; i++)
- s->lvt[i] = 1 << 16; /* mask LVT */
- s->esr = 0;
- memset(s->icr, 0, sizeof(s->icr));
- s->divide_conf = 0;
- s->count_shift = 0;
- s->initial_count = 0;
- s->initial_count_load_time = 0;
- s->next_time = 0;
- s->wait_for_sipi = 1;
-
- qemu_del_timer(s->timer);
-}
-
static void apic_startup(APICState *s, int vector_num)
{
s->sipi_vector = vector_num;
@@ -892,96 +748,6 @@ static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
}
}
-/* This function is only used for old state version 1 and 2 */
-static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
-{
- APICState *s = opaque;
- int i;
-
- if (version_id > 2)
- return -EINVAL;
-
- /* XXX: what if the base changes? (registered memory regions) */
- qemu_get_be32s(f, &s->apicbase);
- qemu_get_8s(f, &s->id);
- qemu_get_8s(f, &s->arb_id);
- qemu_get_8s(f, &s->tpr);
- qemu_get_be32s(f, &s->spurious_vec);
- qemu_get_8s(f, &s->log_dest);
- qemu_get_8s(f, &s->dest_mode);
- for (i = 0; i < 8; i++) {
- qemu_get_be32s(f, &s->isr[i]);
- qemu_get_be32s(f, &s->tmr[i]);
- qemu_get_be32s(f, &s->irr[i]);
- }
- for (i = 0; i < APIC_LVT_NB; i++) {
- qemu_get_be32s(f, &s->lvt[i]);
- }
- qemu_get_be32s(f, &s->esr);
- qemu_get_be32s(f, &s->icr[0]);
- qemu_get_be32s(f, &s->icr[1]);
- qemu_get_be32s(f, &s->divide_conf);
- s->count_shift=qemu_get_be32(f);
- qemu_get_be32s(f, &s->initial_count);
- s->initial_count_load_time=qemu_get_be64(f);
- s->next_time=qemu_get_be64(f);
-
- if (version_id >= 2)
- qemu_get_timer(f, s->timer);
- return 0;
-}
-
-static const VMStateDescription vmstate_apic = {
- .name = "apic",
- .version_id = 3,
- .minimum_version_id = 3,
- .minimum_version_id_old = 1,
- .load_state_old = apic_load_old,
- .fields = (VMStateField []) {
- VMSTATE_UINT32(apicbase, APICState),
- VMSTATE_UINT8(id, APICState),
- VMSTATE_UINT8(arb_id, APICState),
- VMSTATE_UINT8(tpr, APICState),
- VMSTATE_UINT32(spurious_vec, APICState),
- VMSTATE_UINT8(log_dest, APICState),
- VMSTATE_UINT8(dest_mode, APICState),
- VMSTATE_UINT32_ARRAY(isr, APICState, 8),
- VMSTATE_UINT32_ARRAY(tmr, APICState, 8),
- VMSTATE_UINT32_ARRAY(irr, APICState, 8),
- VMSTATE_UINT32_ARRAY(lvt, APICState, APIC_LVT_NB),
- VMSTATE_UINT32(esr, APICState),
- VMSTATE_UINT32_ARRAY(icr, APICState, 2),
- VMSTATE_UINT32(divide_conf, APICState),
- VMSTATE_INT32(count_shift, APICState),
- VMSTATE_UINT32(initial_count, APICState),
- VMSTATE_INT64(initial_count_load_time, APICState),
- VMSTATE_INT64(next_time, APICState),
- VMSTATE_TIMER(timer, APICState),
- VMSTATE_END_OF_LIST()
- }
-};
-
-static void apic_reset(DeviceState *d)
-{
- APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
- int bsp;
-
- bsp = cpu_is_bsp(s->cpu_env);
- s->apicbase = 0xfee00000 |
- (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
-
- apic_init_reset(d);
-
- if (bsp) {
- /*
- * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
- * time typically by BIOS, so PIC interrupt can be delivered to the
- * processor when local APIC is enabled.
- */
- s->lvt[APIC_LVT_LINT0] = 0x700;
- }
-}
-
static const MemoryRegionOps apic_io_ops = {
.old_mmio = {
.read = { apic_mem_readb, apic_mem_readw, apic_mem_readl, },
@@ -990,26 +756,28 @@ static const MemoryRegionOps apic_io_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static int apic_init1(SysBusDevice *dev)
+static int apic_init(SysBusDevice *dev)
{
APICState *s = FROM_SYSBUS(APICState, dev);
- static int last_apic_idx;
- if (last_apic_idx >= MAX_APICS) {
+ memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic-msi",
+ MSI_SPACE_SIZE);
+
+ s->idx = apic_init_common(s);
+ if (s->idx < 0) {
+ memory_region_destroy(&s->io_memory);
return -1;
}
- memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
- MSI_ADDR_SIZE);
- sysbus_init_mmio_region(dev, &s->io_memory);
s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
- s->idx = last_apic_idx++;
+ s->set_base = apic_set_base;
+ s->set_tpr = apic_set_tpr;
local_apics[s->idx] = s;
return 0;
}
static SysBusDeviceInfo apic_info = {
- .init = apic_init1,
+ .init = apic_init,
.qdev.name = "apic",
.qdev.size = sizeof(APICState),
.qdev.vmsd = &vmstate_apic,
diff --git a/hw/apic_common.c b/hw/apic_common.c
new file mode 100644
index 0000000..7d30356
--- /dev/null
+++ b/hw/apic_common.c
@@ -0,0 +1,215 @@
+/*
+ * APIC support - common bits of emulated and KVM kernel model
+ *
+ * Copyright (c) 2004-2005 Fabrice Bellard
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+#include "apic.h"
+#include "apic_internal.h"
+#include "trace.h"
+
+static int apic_irq_delivered;
+
+void cpu_set_apic_base(DeviceState *d, uint64_t val)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+ trace_cpu_set_apic_base(val);
+
+ if (s) {
+ s->set_base(s, val);
+ }
+}
+
+uint64_t cpu_get_apic_base(DeviceState *d)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+ trace_cpu_get_apic_base(s ? (uint64_t)s->apicbase : 0);
+
+ return s ? s->apicbase : 0;
+}
+
+void cpu_set_apic_tpr(DeviceState *d, uint8_t val)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+ if (s) {
+ s->set_tpr(s, val);
+ }
+}
+
+uint8_t cpu_get_apic_tpr(DeviceState *d)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+ return s ? s->tpr >> 4 : 0;
+}
+
+void apic_set_irq_delivered(int delivered)
+{
+ apic_irq_delivered += delivered;
+
+ trace_apic_set_irq_delivered(apic_irq_delivered);
+}
+
+void apic_reset_irq_delivered(void)
+{
+ trace_apic_reset_irq_delivered(apic_irq_delivered);
+
+ apic_irq_delivered = false;
+}
+
+int apic_get_irq_delivered(void)
+{
+ trace_apic_get_irq_delivered(apic_irq_delivered);
+
+ return apic_irq_delivered;
+}
+
+void apic_init_reset(DeviceState *d)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+ int i;
+
+ if (!s) {
+ return;
+ }
+ s->tpr = 0;
+ s->spurious_vec = 0xff;
+ s->log_dest = 0;
+ s->dest_mode = 0xf;
+ memset(s->isr, 0, sizeof(s->isr));
+ memset(s->tmr, 0, sizeof(s->tmr));
+ memset(s->irr, 0, sizeof(s->irr));
+ for (i = 0; i < APIC_LVT_NB; i++) {
+ s->lvt[i] = APIC_LVT_MASKED;
+ }
+ s->esr = 0;
+ memset(s->icr, 0, sizeof(s->icr));
+ s->divide_conf = 0;
+ s->count_shift = 0;
+ s->initial_count = 0;
+ s->initial_count_load_time = 0;
+ s->next_time = 0;
+ s->wait_for_sipi = 1;
+
+ qemu_del_timer(s->timer);
+}
+
+void apic_reset(DeviceState *d)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+ bool bsp;
+
+ bsp = cpu_is_bsp(s->cpu_env);
+ s->apicbase = 0xfee00000 |
+ (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
+
+ apic_init_reset(d);
+
+ if (bsp) {
+ /*
+ * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
+ * time typically by BIOS, so PIC interrupt can be delivered to the
+ * processor when local APIC is enabled.
+ */
+ s->lvt[APIC_LVT_LINT0] = 0x700;
+ }
+}
+
+/* This function is only used for old state version 1 and 2 */
+static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
+{
+ APICState *s = opaque;
+ int i;
+
+ if (version_id > 2) {
+ return -EINVAL;
+ }
+
+ /* XXX: what if the base changes? (registered memory regions) */
+ qemu_get_be32s(f, &s->apicbase);
+ qemu_get_8s(f, &s->id);
+ qemu_get_8s(f, &s->arb_id);
+ qemu_get_8s(f, &s->tpr);
+ qemu_get_be32s(f, &s->spurious_vec);
+ qemu_get_8s(f, &s->log_dest);
+ qemu_get_8s(f, &s->dest_mode);
+ for (i = 0; i < 8; i++) {
+ qemu_get_be32s(f, &s->isr[i]);
+ qemu_get_be32s(f, &s->tmr[i]);
+ qemu_get_be32s(f, &s->irr[i]);
+ }
+ for (i = 0; i < APIC_LVT_NB; i++) {
+ qemu_get_be32s(f, &s->lvt[i]);
+ }
+ qemu_get_be32s(f, &s->esr);
+ qemu_get_be32s(f, &s->icr[0]);
+ qemu_get_be32s(f, &s->icr[1]);
+ qemu_get_be32s(f, &s->divide_conf);
+ s->count_shift = qemu_get_be32(f);
+ qemu_get_be32s(f, &s->initial_count);
+ s->initial_count_load_time = qemu_get_be64(f);
+ s->next_time = qemu_get_be64(f);
+
+ if (version_id >= 2) {
+ qemu_get_timer(f, s->timer);
+ }
+ return 0;
+}
+
+const VMStateDescription vmstate_apic = {
+ .name = "apic",
+ .version_id = 3,
+ .minimum_version_id = 3,
+ .minimum_version_id_old = 1,
+ .load_state_old = apic_load_old,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(apicbase, APICState),
+ VMSTATE_UINT8(id, APICState),
+ VMSTATE_UINT8(arb_id, APICState),
+ VMSTATE_UINT8(tpr, APICState),
+ VMSTATE_UINT32(spurious_vec, APICState),
+ VMSTATE_UINT8(log_dest, APICState),
+ VMSTATE_UINT8(dest_mode, APICState),
+ VMSTATE_UINT32_ARRAY(isr, APICState, 8),
+ VMSTATE_UINT32_ARRAY(tmr, APICState, 8),
+ VMSTATE_UINT32_ARRAY(irr, APICState, 8),
+ VMSTATE_UINT32_ARRAY(lvt, APICState, APIC_LVT_NB),
+ VMSTATE_UINT32(esr, APICState),
+ VMSTATE_UINT32_ARRAY(icr, APICState, 2),
+ VMSTATE_UINT32(divide_conf, APICState),
+ VMSTATE_INT32(count_shift, APICState),
+ VMSTATE_UINT32(initial_count, APICState),
+ VMSTATE_INT64(initial_count_load_time, APICState),
+ VMSTATE_INT64(next_time, APICState),
+ VMSTATE_TIMER(timer, APICState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+int apic_init_common(APICState *s)
+{
+ static int apic_no;
+
+ if (apic_no >= MAX_APICS) {
+ return -1;
+ }
+ sysbus_init_mmio_region(&s->busdev, &s->io_memory);
+
+ return apic_no++;
+}
diff --git a/hw/apic_internal.h b/hw/apic_internal.h
new file mode 100644
index 0000000..36b45ce
--- /dev/null
+++ b/hw/apic_internal.h
@@ -0,0 +1,108 @@
+/*
+ * APIC support - internal interfaces
+ *
+ * Copyright (c) 2004-2005 Fabrice Bellard
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+#ifndef QEMU_APIC_INTERNAL_H
+#define QEMU_APIC_INTERNAL_H
+
+#include "memory.h"
+#include "sysbus.h"
+#include "qemu-timer.h"
+
+/* APIC Local Vector Table */
+#define APIC_LVT_TIMER 0
+#define APIC_LVT_THERMAL 1
+#define APIC_LVT_PERFORM 2
+#define APIC_LVT_LINT0 3
+#define APIC_LVT_LINT1 4
+#define APIC_LVT_ERROR 5
+#define APIC_LVT_NB 6
+
+/* APIC delivery modes */
+#define APIC_DM_FIXED 0
+#define APIC_DM_LOWPRI 1
+#define APIC_DM_SMI 2
+#define APIC_DM_NMI 4
+#define APIC_DM_INIT 5
+#define APIC_DM_SIPI 6
+#define APIC_DM_EXTINT 7
+
+/* APIC destination mode */
+#define APIC_DESTMODE_FLAT 0xf
+#define APIC_DESTMODE_CLUSTER 1
+
+#define APIC_TRIGGER_EDGE 0
+#define APIC_TRIGGER_LEVEL 1
+
+#define APIC_LVT_TIMER_PERIODIC (1<<17)
+#define APIC_LVT_MASKED (1<<16)
+#define APIC_LVT_LEVEL_TRIGGER (1<<15)
+#define APIC_LVT_REMOTE_IRR (1<<14)
+#define APIC_INPUT_POLARITY (1<<13)
+#define APIC_SEND_PENDING (1<<12)
+
+#define ESR_ILLEGAL_ADDRESS (1 << 7)
+
+#define APIC_SV_DIRECTED_IO (1<<12)
+#define APIC_SV_ENABLE (1<<8)
+
+#define MAX_APICS 255
+
+#define MSI_SPACE_SIZE 0x100000
+
+typedef struct APICState APICState;
+
+struct APICState {
+ SysBusDevice busdev;
+ MemoryRegion io_memory;
+ void *cpu_env;
+ uint32_t apicbase;
+ uint8_t id;
+ uint8_t arb_id;
+ uint8_t tpr;
+ uint32_t spurious_vec;
+ uint8_t log_dest;
+ uint8_t dest_mode;
+ uint32_t isr[8]; /* in service register */
+ uint32_t tmr[8]; /* trigger mode register */
+ uint32_t irr[8]; /* interrupt request register */
+ uint32_t lvt[APIC_LVT_NB];
+ uint32_t esr; /* error register */
+ uint32_t icr[2];
+
+ uint32_t divide_conf;
+ int count_shift;
+ uint32_t initial_count;
+ int64_t initial_count_load_time;
+ int64_t next_time;
+ int idx;
+ QEMUTimer *timer;
+ int sipi_vector;
+ int wait_for_sipi;
+
+ void (*set_base)(APICState *s, uint64_t val);
+ void (*set_tpr)(APICState *s, uint8_t val);
+};
+
+extern const VMStateDescription vmstate_apic;
+
+int apic_init_common(APICState *s);
+void apic_reset(DeviceState *d);
+void apic_set_irq_delivered(int delivered);
+
+#endif /* !QEMU_APIC_INTERNAL_H */
diff --git a/trace-events b/trace-events
index 7f9cec4..e8b17f6 100644
--- a/trace-events
+++ b/trace-events
@@ -97,7 +97,7 @@ apic_mem_writel(uint64_t addr, uint32_t val) "%"PRIx64" = %08x"
# coalescing
apic_reset_irq_delivered(int apic_irq_delivered) "old coalescing %d"
apic_get_irq_delivered(int apic_irq_delivered) "returning coalescing %d"
-apic_set_irq(int apic_irq_delivered) "coalescing %d"
+apic_set_irq_delivered(int apic_irq_delivered) "coalescing %d"
# hw/cs4231.c
cs4231_mem_readl_dreg(uint32_t reg, uint32_t ret) "read dreg %d: 0x%02x"
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 05/16] apic: Open-code timer save/restore
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (3 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 04/16] apic: Factor out core for KVM reuse Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 06/16] i8259: Factor out core for KVM reuse Jan Kiszka
` (10 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
To enable migration between accelerated and non-accelerated APIC models,
we will need to handle the timer saving and restoring specially and can
no longer rely on the automatics of VMSTATE_TIMER. Specifically,
accelerated model will not start any QEMUTimer.
This patch therefore factors out the generic bits into apic_next_timer
and introduces a post-load callback that can be implemented differently
by both models.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/apic.c | 30 ++++++++++++------------------
hw/apic_common.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--
hw/apic_internal.h | 3 +++
3 files changed, 64 insertions(+), 20 deletions(-)
diff --git a/hw/apic.c b/hw/apic.c
index 27b18d6..9b83c0c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -516,25 +516,9 @@ static uint32_t apic_get_current_count(APICState *s)
static void apic_timer_update(APICState *s, int64_t current_time)
{
- int64_t next_time, d;
-
- if (!(s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED)) {
- d = (current_time - s->initial_count_load_time) >>
- s->count_shift;
- if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
- if (!s->initial_count)
- goto no_timer;
- d = ((d / ((uint64_t)s->initial_count + 1)) + 1) * ((uint64_t)s->initial_count + 1);
- } else {
- if (d >= s->initial_count)
- goto no_timer;
- d = (uint64_t)s->initial_count + 1;
- }
- next_time = s->initial_count_load_time + (d << s->count_shift);
- qemu_mod_timer(s->timer, next_time);
- s->next_time = next_time;
+ if (apic_next_timer(s, current_time)) {
+ qemu_mod_timer(s->timer, s->next_time);
} else {
- no_timer:
qemu_del_timer(s->timer);
}
}
@@ -756,6 +740,15 @@ static const MemoryRegionOps apic_io_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+static void apic_post_load(APICState *s)
+{
+ if (s->timer_expiry != -1) {
+ qemu_mod_timer(s->timer, s->timer_expiry);
+ } else {
+ qemu_del_timer(s->timer);
+ }
+}
+
static int apic_init(SysBusDevice *dev)
{
APICState *s = FROM_SYSBUS(APICState, dev);
@@ -772,6 +765,7 @@ static int apic_init(SysBusDevice *dev)
s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
s->set_base = apic_set_base;
s->set_tpr = apic_set_tpr;
+ s->post_load = apic_post_load;
local_apics[s->idx] = s;
return 0;
}
diff --git a/hw/apic_common.c b/hw/apic_common.c
index 7d30356..84a3a27 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -80,6 +80,39 @@ int apic_get_irq_delivered(void)
return apic_irq_delivered;
}
+bool apic_next_timer(APICState *s, int64_t current_time)
+{
+ int64_t d;
+
+ /* We need to store the timer state separately to support APIC
+ * implementations that maintain a non-QEMU timer, e.g. inside the
+ * host kernel. This open-coded state allows us to migrate between
+ * both models. */
+ s->timer_expiry = -1;
+
+ if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED) {
+ return false;
+ }
+
+ d = (current_time - s->initial_count_load_time) >> s->count_shift;
+
+ if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
+ if (!s->initial_count) {
+ return false;
+ }
+ d = ((d / ((uint64_t)s->initial_count + 1)) + 1) *
+ ((uint64_t)s->initial_count + 1);
+ } else {
+ if (d >= s->initial_count) {
+ return false;
+ }
+ d = (uint64_t)s->initial_count + 1;
+ }
+ s->next_time = s->initial_count_load_time + (d << s->count_shift);
+ s->timer_expiry = s->next_time;
+ return true;
+}
+
void apic_init_reset(DeviceState *d)
{
APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
@@ -107,7 +140,10 @@ void apic_init_reset(DeviceState *d)
s->next_time = 0;
s->wait_for_sipi = 1;
- qemu_del_timer(s->timer);
+ if (s->timer) {
+ qemu_del_timer(s->timer);
+ }
+ s->timer_expiry = -1;
}
void apic_reset(DeviceState *d)
@@ -172,12 +208,23 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
return 0;
}
+static int apic_dispatch_post_load(void *opaque, int version_id)
+{
+ APICState *s = opaque;
+
+ if (s->post_load) {
+ s->post_load(s);
+ }
+ return 0;
+}
+
const VMStateDescription vmstate_apic = {
.name = "apic",
.version_id = 3,
.minimum_version_id = 3,
.minimum_version_id_old = 1,
.load_state_old = apic_load_old,
+ .post_load = apic_dispatch_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT32(apicbase, APICState),
VMSTATE_UINT8(id, APICState),
@@ -197,7 +244,7 @@ const VMStateDescription vmstate_apic = {
VMSTATE_UINT32(initial_count, APICState),
VMSTATE_INT64(initial_count_load_time, APICState),
VMSTATE_INT64(next_time, APICState),
- VMSTATE_TIMER(timer, APICState),
+ VMSTATE_INT64(timer_expiry, APICState), /* open-coded timer state */
VMSTATE_END_OF_LIST()
}
};
diff --git a/hw/apic_internal.h b/hw/apic_internal.h
index 36b45ce..b110cf3 100644
--- a/hw/apic_internal.h
+++ b/hw/apic_internal.h
@@ -92,17 +92,20 @@ struct APICState {
int64_t next_time;
int idx;
QEMUTimer *timer;
+ int64_t timer_expiry;
int sipi_vector;
int wait_for_sipi;
void (*set_base)(APICState *s, uint64_t val);
void (*set_tpr)(APICState *s, uint8_t val);
+ void (*post_load)(APICState *s);
};
extern const VMStateDescription vmstate_apic;
int apic_init_common(APICState *s);
void apic_reset(DeviceState *d);
+bool apic_next_timer(APICState *s, int64_t current_time);
void apic_set_irq_delivered(int delivered);
#endif /* !QEMU_APIC_INTERNAL_H */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 06/16] i8259: Factor out core for KVM reuse
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (4 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 05/16] apic: Open-code timer save/restore Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 07/16] ioapic: Convert to memory API Jan Kiszka
` (9 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Analogously to the APIC, we will reuse some parts of the user space
i8259 model for KVM. In this case it is the PicState, vmstate
description, a reset core and some init bits.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.objs | 2 +-
hw/i8259.c | 78 +-------------------------------------
hw/i8259_common.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++
hw/i8259_internal.h | 67 +++++++++++++++++++++++++++++++++
4 files changed, 174 insertions(+), 76 deletions(-)
create mode 100644 hw/i8259_common.c
create mode 100644 hw/i8259_internal.h
diff --git a/Makefile.objs b/Makefile.objs
index 01587c8..5372eec 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -220,7 +220,7 @@ hw-obj-$(CONFIG_APPLESMC) += applesmc.o
hw-obj-$(CONFIG_SMARTCARD) += usb-ccid.o ccid-card-passthru.o
hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o
hw-obj-$(CONFIG_USB_REDIR) += usb-redir.o
-hw-obj-$(CONFIG_I8259) += i8259.o
+hw-obj-$(CONFIG_I8259) += i8259_common.o i8259.o
# PPC devices
hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
diff --git a/hw/i8259.c b/hw/i8259.c
index ab519de..e8a6a9a 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -26,6 +26,7 @@
#include "isa.h"
#include "monitor.h"
#include "qemu-timer.h"
+#include "i8259_internal.h"
/* debug PIC */
//#define DEBUG_PIC
@@ -40,33 +41,6 @@
//#define DEBUG_IRQ_LATENCY
//#define DEBUG_IRQ_COUNT
-struct PicState {
- ISADevice dev;
- uint8_t last_irr; /* edge detection */
- uint8_t irr; /* interrupt request register */
- uint8_t imr; /* interrupt mask register */
- uint8_t isr; /* interrupt service register */
- uint8_t priority_add; /* highest irq priority */
- uint8_t irq_base;
- uint8_t read_reg_select;
- uint8_t poll;
- uint8_t special_mask;
- uint8_t init_state;
- uint8_t auto_eoi;
- uint8_t rotate_on_auto_eoi;
- uint8_t special_fully_nested_mode;
- uint8_t init4; /* true if 4 byte init */
- uint8_t single_mode; /* true if slave pic is not initialized */
- uint8_t elcr; /* PIIX edge/trigger selection*/
- uint8_t elcr_mask;
- qemu_irq int_out[1];
- uint32_t master; /* reflects /SP input pin */
- uint32_t iobase;
- uint32_t elcr_addr;
- MemoryRegion base_io;
- MemoryRegion elcr_io;
-};
-
#if defined(DEBUG_PIC) || defined(DEBUG_IRQ_COUNT)
static int irq_level[16];
#endif
@@ -248,22 +222,7 @@ int pic_read_irq(PicState *s)
static void pic_init_reset(PicState *s)
{
- s->last_irr = 0;
- s->irr = 0;
- s->imr = 0;
- s->isr = 0;
- s->priority_add = 0;
- s->irq_base = 0;
- s->read_reg_select = 0;
- s->poll = 0;
- s->special_mask = 0;
- s->init_state = 0;
- s->auto_eoi = 0;
- s->rotate_on_auto_eoi = 0;
- s->special_fully_nested_mode = 0;
- s->init4 = 0;
- s->single_mode = 0;
- /* Note: ELCR is not reset */
+ pic_reset_internal(s);
pic_update_irq(s);
}
@@ -418,32 +377,6 @@ static uint64_t elcr_ioport_read(void *opaque, target_phys_addr_t addr,
return s->elcr;
}
-static const VMStateDescription vmstate_pic = {
- .name = "i8259",
- .version_id = 1,
- .minimum_version_id = 1,
- .minimum_version_id_old = 1,
- .fields = (VMStateField[]) {
- VMSTATE_UINT8(last_irr, PicState),
- VMSTATE_UINT8(irr, PicState),
- VMSTATE_UINT8(imr, PicState),
- VMSTATE_UINT8(isr, PicState),
- VMSTATE_UINT8(priority_add, PicState),
- VMSTATE_UINT8(irq_base, PicState),
- VMSTATE_UINT8(read_reg_select, PicState),
- VMSTATE_UINT8(poll, PicState),
- VMSTATE_UINT8(special_mask, PicState),
- VMSTATE_UINT8(init_state, PicState),
- VMSTATE_UINT8(auto_eoi, PicState),
- VMSTATE_UINT8(rotate_on_auto_eoi, PicState),
- VMSTATE_UINT8(special_fully_nested_mode, PicState),
- VMSTATE_UINT8(init4, PicState),
- VMSTATE_UINT8(single_mode, PicState),
- VMSTATE_UINT8(elcr, PicState),
- VMSTATE_END_OF_LIST()
- }
-};
-
static const MemoryRegionOps pic_base_ioport_ops = {
.read = pic_ioport_read,
.write = pic_ioport_write,
@@ -469,16 +402,11 @@ static int pic_initfn(ISADevice *dev)
memory_region_init_io(&s->base_io, &pic_base_ioport_ops, s, "pic", 2);
memory_region_init_io(&s->elcr_io, &pic_elcr_ioport_ops, s, "elcr", 1);
- isa_register_ioport(NULL, &s->base_io, s->iobase);
- if (s->elcr_addr != -1) {
- isa_register_ioport(NULL, &s->elcr_io, s->elcr_addr);
- }
+ pic_init_common(s);
qdev_init_gpio_out(&dev->qdev, s->int_out, ARRAY_SIZE(s->int_out));
qdev_init_gpio_in(&dev->qdev, pic_set_irq, 8);
- qdev_set_legacy_instance_id(&dev->qdev, s->iobase, 1);
-
return 0;
}
diff --git a/hw/i8259_common.c b/hw/i8259_common.c
new file mode 100644
index 0000000..9d2fbc3
--- /dev/null
+++ b/hw/i8259_common.c
@@ -0,0 +1,103 @@
+/*
+ * QEMU 8259 - common bits of emulated and KVM kernel model
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "pc.h"
+#include "i8259_internal.h"
+
+void pic_reset_internal(PicState *s)
+{
+ s->last_irr = 0;
+ s->irr = 0;
+ s->imr = 0;
+ s->isr = 0;
+ s->priority_add = 0;
+ s->irq_base = 0;
+ s->read_reg_select = 0;
+ s->poll = 0;
+ s->special_mask = 0;
+ s->init_state = 0;
+ s->auto_eoi = 0;
+ s->rotate_on_auto_eoi = 0;
+ s->special_fully_nested_mode = 0;
+ s->init4 = 0;
+ s->single_mode = 0;
+ /* Note: ELCR is not reset */
+}
+
+static void pic_dispatch_pre_save(void *opaque)
+{
+ PicState *s = opaque;
+
+ if (s->pre_save) {
+ s->pre_save(s);
+ }
+}
+
+static int pic_dispatch_post_load(void *opaque, int version_id)
+{
+ PicState *s = opaque;
+
+ if (s->post_load) {
+ s->post_load(s);
+ }
+ return 0;
+}
+
+const VMStateDescription vmstate_pic = {
+ .name = "i8259",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .pre_save = pic_dispatch_pre_save,
+ .post_load = pic_dispatch_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(last_irr, PicState),
+ VMSTATE_UINT8(irr, PicState),
+ VMSTATE_UINT8(imr, PicState),
+ VMSTATE_UINT8(isr, PicState),
+ VMSTATE_UINT8(priority_add, PicState),
+ VMSTATE_UINT8(irq_base, PicState),
+ VMSTATE_UINT8(read_reg_select, PicState),
+ VMSTATE_UINT8(poll, PicState),
+ VMSTATE_UINT8(special_mask, PicState),
+ VMSTATE_UINT8(init_state, PicState),
+ VMSTATE_UINT8(auto_eoi, PicState),
+ VMSTATE_UINT8(rotate_on_auto_eoi, PicState),
+ VMSTATE_UINT8(special_fully_nested_mode, PicState),
+ VMSTATE_UINT8(init4, PicState),
+ VMSTATE_UINT8(single_mode, PicState),
+ VMSTATE_UINT8(elcr, PicState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+void pic_init_common(PicState *s)
+{
+ isa_register_ioport(NULL, &s->base_io, s->iobase);
+ if (s->elcr_addr != -1) {
+ isa_register_ioport(NULL, &s->elcr_io, s->elcr_addr);
+ }
+
+ qdev_set_legacy_instance_id(&s->dev.qdev, s->iobase, 1);
+}
diff --git a/hw/i8259_internal.h b/hw/i8259_internal.h
new file mode 100644
index 0000000..49e8bbd
--- /dev/null
+++ b/hw/i8259_internal.h
@@ -0,0 +1,67 @@
+/*
+ * QEMU 8259 - internal interfaces
+ *
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_I8259_INTERNAL_H
+#define QEMU_I8259_INTERNAL_H
+
+#include "hw.h"
+#include "pc.h"
+#include "isa.h"
+
+struct PicState {
+ ISADevice dev;
+ uint8_t last_irr; /* edge detection */
+ uint8_t irr; /* interrupt request register */
+ uint8_t imr; /* interrupt mask register */
+ uint8_t isr; /* interrupt service register */
+ uint8_t priority_add; /* highest irq priority */
+ uint8_t irq_base;
+ uint8_t read_reg_select;
+ uint8_t poll;
+ uint8_t special_mask;
+ uint8_t init_state;
+ uint8_t auto_eoi;
+ uint8_t rotate_on_auto_eoi;
+ uint8_t special_fully_nested_mode;
+ uint8_t init4; /* true if 4 byte init */
+ uint8_t single_mode; /* true if slave pic is not initialized */
+ uint8_t elcr; /* PIIX edge/trigger selection*/
+ uint8_t elcr_mask;
+ qemu_irq int_out[1];
+ uint32_t master; /* reflects /SP input pin */
+ uint32_t iobase;
+ uint32_t elcr_addr;
+ MemoryRegion base_io;
+ MemoryRegion elcr_io;
+
+ void (*pre_save)(PicState *s);
+ void (*post_load)(PicState *s);
+};
+
+extern const VMStateDescription vmstate_pic;
+
+void pic_init_common(PicState *s);
+void pic_reset_internal(PicState *s);
+
+#endif /* !QEMU_I8259_INTERNAL_H */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 07/16] ioapic: Convert to memory API
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (5 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 06/16] i8259: Factor out core for KVM reuse Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 08/16] ioapic: Reject non-dword accesses to IOWIN register Jan Kiszka
` (8 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
This maintains the old imprecise access size handling.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 28 +++++++++++-----------------
1 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 61991d7..56b1612 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -86,6 +86,7 @@ typedef struct IOAPICState IOAPICState;
struct IOAPICState {
SysBusDevice busdev;
+ MemoryRegion io_memory;
uint8_t id;
uint8_t ioregsel;
uint32_t irr;
@@ -195,7 +196,8 @@ void ioapic_eoi_broadcast(int vector)
}
}
-static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
+static uint64_t
+ioapic_mem_read(void *opaque, target_phys_addr_t addr, unsigned int size)
{
IOAPICState *s = opaque;
int index;
@@ -234,7 +236,8 @@ static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
}
static void
-ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+ioapic_mem_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+ unsigned int size)
{
IOAPICState *s = opaque;
int index;
@@ -309,32 +312,23 @@ static void ioapic_reset(DeviceState *d)
}
}
-static CPUReadMemoryFunc * const ioapic_mem_read[3] = {
- ioapic_mem_readl,
- ioapic_mem_readl,
- ioapic_mem_readl,
-};
-
-static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {
- ioapic_mem_writel,
- ioapic_mem_writel,
- ioapic_mem_writel,
+static const MemoryRegionOps ioapic_io_ops = {
+ .read = ioapic_mem_read,
+ .write = ioapic_mem_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
};
static int ioapic_init1(SysBusDevice *dev)
{
IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
- int io_memory;
static int ioapic_no;
if (ioapic_no >= MAX_IOAPICS) {
return -1;
}
- io_memory = cpu_register_io_memory(ioapic_mem_read,
- ioapic_mem_write, s,
- DEVICE_NATIVE_ENDIAN);
- sysbus_init_mmio(dev, 0x1000, io_memory);
+ memory_region_init_io(&s->io_memory, &ioapic_io_ops, s, "ioapic", 0x1000);
+ sysbus_init_mmio_region(dev, &s->io_memory);
qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 08/16] ioapic: Reject non-dword accesses to IOWIN register
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (6 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 07/16] ioapic: Convert to memory API Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 09/16] ioapic: Factor out core for KVM reuse Jan Kiszka
` (7 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Aligns the model with the spec.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 56b1612..eb75766 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -208,6 +208,9 @@ ioapic_mem_read(void *opaque, target_phys_addr_t addr, unsigned int size)
val = s->ioregsel;
break;
case IOAPIC_IOWIN:
+ if (size != 4) {
+ break;
+ }
switch (s->ioregsel) {
case IOAPIC_REG_ID:
val = s->id << IOAPIC_ID_SHIFT;
@@ -247,6 +250,9 @@ ioapic_mem_write(void *opaque, target_phys_addr_t addr, uint64_t val,
s->ioregsel = val;
break;
case IOAPIC_IOWIN:
+ if (size != 4) {
+ break;
+ }
DPRINTF("write: %08x = %08x\n", s->ioregsel, val);
switch (s->ioregsel) {
case IOAPIC_REG_ID:
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 09/16] ioapic: Factor out core for KVM reuse
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (7 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 08/16] ioapic: Reject non-dword accesses to IOWIN register Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 10/16] memory: Introduce memory_region_init_reservation Jan Kiszka
` (6 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
KVM will share the IOAPICState, the vmstate, the reset logic and certain
init parts with the user space model.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.target | 2 +-
hw/ioapic.c | 108 ++++---------------------------------------------
hw/ioapic_common.c | 89 +++++++++++++++++++++++++++++++++++++++++
hw/ioapic_internal.h | 93 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 192 insertions(+), 100 deletions(-)
create mode 100644 hw/ioapic_common.c
create mode 100644 hw/ioapic_internal.h
diff --git a/Makefile.target b/Makefile.target
index 7bb6b13..4cd3c0e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -226,7 +226,7 @@ obj-$(CONFIG_IVSHMEM) += ivshmem.o
# Hardware support
obj-i386-y += vga.o
obj-i386-y += mc146818rtc.o pc.o
-obj-i386-y += cirrus_vga.o sga.o apic_common.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o sga.o apic_common.o apic.o ioapic_common.o ioapic.o piix_pci.o
obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += debugcon.o multiboot.o
diff --git a/hw/ioapic.c b/hw/ioapic.c
index eb75766..8876d5d 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -24,9 +24,7 @@
#include "pc.h"
#include "apic.h"
#include "ioapic.h"
-#include "qemu-timer.h"
-#include "host-utils.h"
-#include "sysbus.h"
+#include "ioapic_internal.h"
//#define DEBUG_IOAPIC
@@ -37,62 +35,6 @@
#define DPRINTF(fmt, ...)
#endif
-#define MAX_IOAPICS 1
-
-#define IOAPIC_VERSION 0x11
-
-#define IOAPIC_LVT_DEST_SHIFT 56
-#define IOAPIC_LVT_MASKED_SHIFT 16
-#define IOAPIC_LVT_TRIGGER_MODE_SHIFT 15
-#define IOAPIC_LVT_REMOTE_IRR_SHIFT 14
-#define IOAPIC_LVT_POLARITY_SHIFT 13
-#define IOAPIC_LVT_DELIV_STATUS_SHIFT 12
-#define IOAPIC_LVT_DEST_MODE_SHIFT 11
-#define IOAPIC_LVT_DELIV_MODE_SHIFT 8
-
-#define IOAPIC_LVT_MASKED (1 << IOAPIC_LVT_MASKED_SHIFT)
-#define IOAPIC_LVT_REMOTE_IRR (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT)
-
-#define IOAPIC_TRIGGER_EDGE 0
-#define IOAPIC_TRIGGER_LEVEL 1
-
-/*io{apic,sapic} delivery mode*/
-#define IOAPIC_DM_FIXED 0x0
-#define IOAPIC_DM_LOWEST_PRIORITY 0x1
-#define IOAPIC_DM_PMI 0x2
-#define IOAPIC_DM_NMI 0x4
-#define IOAPIC_DM_INIT 0x5
-#define IOAPIC_DM_SIPI 0x6
-#define IOAPIC_DM_EXTINT 0x7
-#define IOAPIC_DM_MASK 0x7
-
-#define IOAPIC_VECTOR_MASK 0xff
-
-#define IOAPIC_IOREGSEL 0x00
-#define IOAPIC_IOWIN 0x10
-
-#define IOAPIC_REG_ID 0x00
-#define IOAPIC_REG_VER 0x01
-#define IOAPIC_REG_ARB 0x02
-#define IOAPIC_REG_REDTBL_BASE 0x10
-#define IOAPIC_ID 0x00
-
-#define IOAPIC_ID_SHIFT 24
-#define IOAPIC_ID_MASK 0xf
-
-#define IOAPIC_VER_ENTRIES_SHIFT 16
-
-typedef struct IOAPICState IOAPICState;
-
-struct IOAPICState {
- SysBusDevice busdev;
- MemoryRegion io_memory;
- uint8_t id;
- uint8_t ioregsel;
- uint32_t irr;
- uint64_t ioredtbl[IOAPIC_NUM_PINS];
-};
-
static IOAPICState *ioapics[MAX_IOAPICS];
static void ioapic_service(IOAPICState *s)
@@ -278,44 +220,11 @@ ioapic_mem_write(void *opaque, target_phys_addr_t addr, uint64_t val,
}
}
-static int ioapic_post_load(void *opaque, int version_id)
-{
- IOAPICState *s = opaque;
-
- if (version_id == 1) {
- /* set sane value */
- s->irr = 0;
- }
- return 0;
-}
-
-static const VMStateDescription vmstate_ioapic = {
- .name = "ioapic",
- .version_id = 3,
- .post_load = ioapic_post_load,
- .minimum_version_id = 1,
- .minimum_version_id_old = 1,
- .fields = (VMStateField[]) {
- VMSTATE_UINT8(id, IOAPICState),
- VMSTATE_UINT8(ioregsel, IOAPICState),
- VMSTATE_UNUSED_V(2, 8), /* to account for qemu-kvm's v2 format */
- VMSTATE_UINT32_V(irr, IOAPICState, 2),
- VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
- VMSTATE_END_OF_LIST()
- }
-};
-
static void ioapic_reset(DeviceState *d)
{
IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
- int i;
- s->id = 0;
- s->ioregsel = 0;
- s->irr = 0;
- for (i = 0; i < IOAPIC_NUM_PINS; i++) {
- s->ioredtbl[i] = 1 << IOAPIC_LVT_MASKED_SHIFT;
- }
+ ioapic_reset_internal(s);
}
static const MemoryRegionOps ioapic_io_ops = {
@@ -327,18 +236,19 @@ static const MemoryRegionOps ioapic_io_ops = {
static int ioapic_init1(SysBusDevice *dev)
{
IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
- static int ioapic_no;
+ int ioapic_no;
- if (ioapic_no >= MAX_IOAPICS) {
+ memory_region_init_io(&s->io_memory, &ioapic_io_ops, s, "ioapic", 0x1000);
+
+ ioapic_no = ioapic_init_common(s);
+ if (ioapic_no < 0) {
+ memory_region_destroy(&s->io_memory);
return -1;
}
- memory_region_init_io(&s->io_memory, &ioapic_io_ops, s, "ioapic", 0x1000);
- sysbus_init_mmio_region(dev, &s->io_memory);
-
qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
- ioapics[ioapic_no++] = s;
+ ioapics[ioapic_no] = s;
return 0;
}
diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
new file mode 100644
index 0000000..7958093
--- /dev/null
+++ b/hw/ioapic_common.c
@@ -0,0 +1,89 @@
+/*
+ * IOAPIC emulation logic - common bits of emulated and KVM kernel model
+ *
+ * Copyright (c) 2004-2005 Fabrice Bellard
+ * Copyright (c) 2009 Xiantao Zhang, Intel
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "ioapic.h"
+#include "ioapic_internal.h"
+#include "sysbus.h"
+
+void ioapic_reset_internal(IOAPICState *s)
+{
+ int i;
+
+ s->id = 0;
+ s->ioregsel = 0;
+ s->irr = 0;
+ for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+ s->ioredtbl[i] = 1 << IOAPIC_LVT_MASKED_SHIFT;
+ }
+}
+
+static void ioapic_dispatch_pre_save(void *opaque)
+{
+ IOAPICState *s = opaque;
+
+ if (s->pre_save) {
+ s->pre_save(s);
+ }
+}
+
+static int ioapic_post_load(void *opaque, int version_id)
+{
+ IOAPICState *s = opaque;
+
+ if (version_id == 1) {
+ /* set sane value */
+ s->irr = 0;
+ }
+ if (s->post_load) {
+ s->post_load(s);
+ }
+ return 0;
+}
+
+const VMStateDescription vmstate_ioapic = {
+ .name = "ioapic",
+ .version_id = 3,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .pre_save = ioapic_dispatch_pre_save,
+ .post_load = ioapic_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(id, IOAPICState),
+ VMSTATE_UINT8(ioregsel, IOAPICState),
+ VMSTATE_UNUSED_V(2, 8), /* to account for qemu-kvm's v2 format */
+ VMSTATE_UINT32_V(irr, IOAPICState, 2),
+ VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+int ioapic_init_common(IOAPICState *s)
+{
+ static int ioapic_no;
+
+ if (ioapic_no >= MAX_IOAPICS) {
+ return -1;
+ }
+
+ sysbus_init_mmio_region(&s->busdev, &s->io_memory);
+
+ return ioapic_no++;
+}
diff --git a/hw/ioapic_internal.h b/hw/ioapic_internal.h
new file mode 100644
index 0000000..bda3608
--- /dev/null
+++ b/hw/ioapic_internal.h
@@ -0,0 +1,93 @@
+/*
+ * IOAPIC emulation logic - internal interfaces
+ *
+ * Copyright (c) 2004-2005 Fabrice Bellard
+ * Copyright (c) 2009 Xiantao Zhang, Intel
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_IOAPIC_INTERNAL_H
+#define QEMU_IOAPIC_INTERNAL_H
+
+#include "hw.h"
+#include "memory.h"
+#include "sysbus.h"
+
+#define MAX_IOAPICS 1
+
+#define IOAPIC_VERSION 0x11
+
+#define IOAPIC_LVT_DEST_SHIFT 56
+#define IOAPIC_LVT_MASKED_SHIFT 16
+#define IOAPIC_LVT_TRIGGER_MODE_SHIFT 15
+#define IOAPIC_LVT_REMOTE_IRR_SHIFT 14
+#define IOAPIC_LVT_POLARITY_SHIFT 13
+#define IOAPIC_LVT_DELIV_STATUS_SHIFT 12
+#define IOAPIC_LVT_DEST_MODE_SHIFT 11
+#define IOAPIC_LVT_DELIV_MODE_SHIFT 8
+
+#define IOAPIC_LVT_MASKED (1 << IOAPIC_LVT_MASKED_SHIFT)
+#define IOAPIC_LVT_REMOTE_IRR (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT)
+
+#define IOAPIC_TRIGGER_EDGE 0
+#define IOAPIC_TRIGGER_LEVEL 1
+
+/*io{apic,sapic} delivery mode*/
+#define IOAPIC_DM_FIXED 0x0
+#define IOAPIC_DM_LOWEST_PRIORITY 0x1
+#define IOAPIC_DM_PMI 0x2
+#define IOAPIC_DM_NMI 0x4
+#define IOAPIC_DM_INIT 0x5
+#define IOAPIC_DM_SIPI 0x6
+#define IOAPIC_DM_EXTINT 0x7
+#define IOAPIC_DM_MASK 0x7
+
+#define IOAPIC_VECTOR_MASK 0xff
+
+#define IOAPIC_IOREGSEL 0x00
+#define IOAPIC_IOWIN 0x10
+
+#define IOAPIC_REG_ID 0x00
+#define IOAPIC_REG_VER 0x01
+#define IOAPIC_REG_ARB 0x02
+#define IOAPIC_REG_REDTBL_BASE 0x10
+#define IOAPIC_ID 0x00
+
+#define IOAPIC_ID_SHIFT 24
+#define IOAPIC_ID_MASK 0xf
+
+#define IOAPIC_VER_ENTRIES_SHIFT 16
+
+typedef struct IOAPICState IOAPICState;
+
+struct IOAPICState {
+ SysBusDevice busdev;
+ MemoryRegion io_memory;
+ uint8_t id;
+ uint8_t ioregsel;
+ uint32_t irr;
+ uint64_t ioredtbl[IOAPIC_NUM_PINS];
+
+ void (*pre_save)(IOAPICState *s);
+ void (*post_load)(IOAPICState *s);
+};
+
+extern const VMStateDescription vmstate_ioapic;
+
+int ioapic_init_common(IOAPICState *s);
+void ioapic_reset_internal(IOAPICState *s);
+
+#endif /* !QEMU_IOAPIC_INTERNAL_H */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 10/16] memory: Introduce memory_region_init_reservation
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (8 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 09/16] ioapic: Factor out core for KVM reuse Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-04 13:20 ` Avi Kivity
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support Jan Kiszka
` (5 subsequent siblings)
15 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Introduce a memory region type that can reserve I/O space. Such regions
are useful for modeling I/O that is only handled outside of QEMU, i.e.
in the context of an accelerator like KVM. Any access to such a region
from QEMU is a bug and will be reported as such.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
memory.c | 35 +++++++++++++++++++++++++++++++++++
memory.h | 15 +++++++++++++++
2 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/memory.c b/memory.c
index dc5e35d..a40990c 100644
--- a/memory.c
+++ b/memory.c
@@ -1003,6 +1003,41 @@ void memory_region_init_rom_device(MemoryRegion *mr,
mr->backend_registered = true;
}
+static uint64_t invalid_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
+{
+ MemoryRegion *mr = opaque;
+
+ fprintf(stderr, "Invalid read from memory region %s\n", mr->name);
+ abort();
+}
+
+static void invalid_write(void *opaque, target_phys_addr_t addr, uint64_t data,
+ unsigned size)
+{
+ MemoryRegion *mr = opaque;
+
+ fprintf(stderr, "Invalid write to memory region %s\n", mr->name);
+ abort();
+}
+
+static const MemoryRegionOps reservation_ops = {
+ .read = invalid_read,
+ .write = invalid_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+void memory_region_init_reservation(MemoryRegion *mr,
+ const char *name,
+ uint64_t size)
+{
+ memory_region_init(mr, name, size);
+ mr->ops = &reservation_ops;
+ mr->opaque = mr;
+ mr->terminates = true;
+ mr->backend_registered = false;
+}
+
void memory_region_destroy(MemoryRegion *mr)
{
assert(QTAILQ_EMPTY(&mr->subregions));
diff --git a/memory.h b/memory.h
index d5b47da..4ff441f 100644
--- a/memory.h
+++ b/memory.h
@@ -242,6 +242,21 @@ void memory_region_init_rom_device(MemoryRegion *mr,
uint64_t size);
/**
+ * memory_region_init_reservation: Initialize a memory region that reserves
+ * I/O space.
+ *
+ * A reservation region primariy serves debugging purposes. It claims I/O
+ * space that is not supposed to be handled by QEMU itself. Any access via
+ * the memory API will cause an abort().
+ *
+ * @mr: the #MemoryRegion to be initialized
+ * @name: used for debugging; not visible to the user or ABI
+ * @size: size of the region.
+ */
+void memory_region_init_reservation(MemoryRegion *mr,
+ const char *name,
+ uint64_t size);
+/**
* memory_region_destroy: Destroy a memory region and relaim all resources.
*
* @mr: the region to be destroyed. May not currently be a subregion
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 10/16] memory: Introduce memory_region_init_reservation
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 10/16] memory: Introduce memory_region_init_reservation Jan Kiszka
@ 2011-12-04 13:20 ` Avi Kivity
2011-12-04 13:24 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:20 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Introduce a memory region type that can reserve I/O space. Such regions
> are useful for modeling I/O that is only handled outside of QEMU, i.e.
> in the context of an accelerator like KVM. Any access to such a region
> from QEMU is a bug and will be reported as such.
This is guest triggerable (DMA into the region), so abort() is too drastic.
> +void memory_region_init_reservation(MemoryRegion *mr,
> + const char *name,
> + uint64_t size)
> +{
> + memory_region_init(mr, name, size);
> + mr->ops = &reservation_ops;
> + mr->opaque = mr;
> + mr->terminates = true;
> + mr->backend_registered = false;
> +}
Just calling memory_region_init_io() is simpler, no?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 10/16] memory: Introduce memory_region_init_reservation
2011-12-04 13:20 ` Avi Kivity
@ 2011-12-04 13:24 ` Jan Kiszka
0 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 13:24 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]
On 2011-12-04 14:20, Avi Kivity wrote:
> On 12/03/2011 01:17 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Introduce a memory region type that can reserve I/O space. Such regions
>> are useful for modeling I/O that is only handled outside of QEMU, i.e.
>> in the context of an accelerator like KVM. Any access to such a region
>> from QEMU is a bug and will be reported as such.
>
> This is guest triggerable (DMA into the region), so abort() is too drastic.
Mmh, true. Will turn it into a print-once warning.
>
>> +void memory_region_init_reservation(MemoryRegion *mr,
>> + const char *name,
>> + uint64_t size)
>> +{
>> + memory_region_init(mr, name, size);
>> + mr->ops = &reservation_ops;
>> + mr->opaque = mr;
>> + mr->terminates = true;
>> + mr->backend_registered = false;
>> +}
>
> Just calling memory_region_init_io() is simpler, no?
Yep.
Thanks,
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (9 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 10/16] memory: Introduce memory_region_init_reservation Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-04 13:23 ` Avi Kivity
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 12/16] kvm: x86: Establish IRQ0 override control Jan Kiszka
` (4 subsequent siblings)
15 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Add the basic infrastructure to active in-kernel irqchip support, inject
interrupts into these models, and maintain IRQ routes.
Routing is optional and depends on the host arch supporting
KVM_CAP_IRQ_ROUTING. When it's not available on x86, we loose the HPET
as we can't route GSI0 to IOAPIC pin 2.
In-kernel irqchip support will once be controlled by the machine
property 'kernel_irqchip', but this is not yet wired up.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
kvm-all.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++++
kvm.h | 8 +++
target-i386/kvm.c | 11 ++++
3 files changed, 168 insertions(+), 0 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index e7faf5c..a85e14f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -76,6 +76,13 @@ struct KVMState
int pit_in_kernel;
int xsave, xcrs;
int many_ioeventfds;
+ int irqchip_inject_ioctl;
+#ifdef KVM_CAP_IRQ_ROUTING
+ struct kvm_irq_routing *irq_routes;
+ int nr_allocated_irq_routes;
+ uint32_t *used_gsi_bitmap;
+ unsigned int max_gsi;
+#endif
};
KVMState *kvm_state;
@@ -692,6 +699,138 @@ static void kvm_handle_interrupt(CPUState *env, int mask)
}
}
+int kvm_irqchip_set_irq(KVMState *s, int irq, int level)
+{
+ struct kvm_irq_level event;
+ int ret;
+
+ assert(s->irqchip_in_kernel);
+
+ event.level = level;
+ event.irq = irq;
+ ret = kvm_vm_ioctl(s, s->irqchip_inject_ioctl, &event);
+ if (ret < 0) {
+ perror("kvm_set_irqchip_line");
+ abort();
+ }
+
+ return (s->irqchip_inject_ioctl == KVM_IRQ_LINE) ? 1 : event.status;
+}
+
+#ifdef KVM_CAP_IRQ_ROUTING
+static void set_gsi(KVMState *s, unsigned int gsi)
+{
+ assert(gsi < s->max_gsi);
+
+ s->used_gsi_bitmap[gsi / 32] |= 1U << (gsi % 32);
+}
+
+static void kvm_init_irq_routing(KVMState *s)
+{
+ int gsi_count;
+
+ gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING);
+ if (gsi_count > 0) {
+ unsigned int gsi_bits, i;
+
+ /* Round up so we can search ints using ffs */
+ gsi_bits = (gsi_count + 31) / 32;
+ s->used_gsi_bitmap = g_malloc0(gsi_bits / 8);
+ s->max_gsi = gsi_bits;
+
+ /* Mark any over-allocated bits as already in use */
+ for (i = gsi_count; i < gsi_bits; i++) {
+ set_gsi(s, i);
+ }
+ }
+
+ s->irq_routes = g_malloc0(sizeof(*s->irq_routes));
+ s->nr_allocated_irq_routes = 0;
+
+ kvm_arch_init_irq_routing(s);
+}
+
+static void kvm_add_routing_entry(KVMState *s,
+ struct kvm_irq_routing_entry *entry)
+{
+ struct kvm_irq_routing_entry *new;
+ int n, size;
+
+ if (s->irq_routes->nr == s->nr_allocated_irq_routes) {
+ n = s->nr_allocated_irq_routes * 2;
+ if (n < 64) {
+ n = 64;
+ }
+ size = sizeof(struct kvm_irq_routing);
+ size += n * sizeof(*new);
+ s->irq_routes = g_realloc(s->irq_routes, size);
+ s->nr_allocated_irq_routes = n;
+ }
+ n = s->irq_routes->nr++;
+ new = &s->irq_routes->entries[n];
+ memset(new, 0, sizeof(*new));
+ new->gsi = entry->gsi;
+ new->type = entry->type;
+ new->flags = entry->flags;
+ new->u = entry->u;
+
+ set_gsi(s, entry->gsi);
+}
+
+void kvm_irqchip_add_route(KVMState *s, int irq, int irqchip, int pin)
+{
+ struct kvm_irq_routing_entry e;
+
+ e.gsi = irq;
+ e.type = KVM_IRQ_ROUTING_IRQCHIP;
+ e.flags = 0;
+ e.u.irqchip.irqchip = irqchip;
+ e.u.irqchip.pin = pin;
+ kvm_add_routing_entry(s, &e);
+}
+
+int kvm_irqchip_commit_routes(KVMState *s)
+{
+ s->irq_routes->flags = 0;
+ return kvm_vm_ioctl(s, KVM_SET_GSI_ROUTING, s->irq_routes);
+}
+
+#else /* !KVM_CAP_IRQ_ROUTING */
+
+static void kvm_init_irq_routing(KVMState *s)
+{
+}
+#endif /* !KVM_CAP_IRQ_ROUTING */
+
+static int kvm_irqchip_create(KVMState *s)
+{
+ QemuOptsList *list = qemu_find_opts("machine");
+ int ret;
+
+ if (QTAILQ_EMPTY(&list->head) ||
+ !qemu_opt_get_bool(QTAILQ_FIRST(&list->head),
+ "kernel_irqchip", false) ||
+ !kvm_check_extension(s, KVM_CAP_IRQCHIP)) {
+ return 0;
+ }
+
+ ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
+ if (ret < 0) {
+ fprintf(stderr, "Create kernel irqchip failed\n");
+ return ret;
+ }
+
+ s->irqchip_inject_ioctl = KVM_IRQ_LINE;
+ if (kvm_check_extension(s, KVM_CAP_IRQ_INJECT_STATUS)) {
+ s->irqchip_inject_ioctl = KVM_IRQ_LINE_STATUS;
+ }
+ s->irqchip_in_kernel = 1;
+
+ kvm_init_irq_routing(s);
+
+ return 0;
+}
+
int kvm_init(void)
{
static const char upgrade_note[] =
@@ -786,6 +925,11 @@ int kvm_init(void)
goto err;
}
+ ret = kvm_irqchip_create(s);
+ if (ret < 0) {
+ goto err;
+ }
+
kvm_state = s;
cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
@@ -1111,6 +1255,11 @@ int kvm_has_many_ioeventfds(void)
return kvm_state->many_ioeventfds;
}
+int kvm_has_gsi_routing(void)
+{
+ return kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING);
+}
+
void kvm_setup_guest_memory(void *start, size_t size)
{
if (!kvm_has_sync_mmu()) {
diff --git a/kvm.h b/kvm.h
index 243b063..0d6c453 100644
--- a/kvm.h
+++ b/kvm.h
@@ -51,6 +51,7 @@ int kvm_has_debugregs(void);
int kvm_has_xsave(void);
int kvm_has_xcrs(void);
int kvm_has_many_ioeventfds(void);
+int kvm_has_gsi_routing(void);
#ifdef NEED_CPU_H
int kvm_init_vcpu(CPUState *env);
@@ -124,6 +125,13 @@ void kvm_arch_reset_vcpu(CPUState *env);
int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr);
int kvm_arch_on_sigbus(int code, void *addr);
+void kvm_arch_init_irq_routing(KVMState *s);
+
+int kvm_irqchip_set_irq(KVMState *s, int irq, int level);
+
+void kvm_irqchip_add_route(KVMState *s, int gsi, int irqchip, int pin);
+int kvm_irqchip_commit_routes(KVMState *s);
+
struct kvm_guest_debug;
struct kvm_debug_exit_arch;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9f07786..232c897 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1864,3 +1864,14 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
return !(env->cr[0] & CR0_PE_MASK) ||
((env->segs[R_CS].selector & 3) != 3);
}
+
+void kvm_arch_init_irq_routing(KVMState *s)
+{
+ if (!kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) {
+ /* If kernel can't do irq routing, interrupt source
+ * override 0->2 cannot be set up as required by HPET.
+ * So we have to disable it.
+ */
+ no_hpet = 1;
+ }
+}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support Jan Kiszka
@ 2011-12-04 13:23 ` Avi Kivity
2011-12-04 13:27 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:23 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Add the basic infrastructure to active in-kernel irqchip support, inject
> interrupts into these models, and maintain IRQ routes.
>
> Routing is optional and depends on the host arch supporting
> KVM_CAP_IRQ_ROUTING. When it's not available on x86, we loose the HPET
lose
> as we can't route GSI0 to IOAPIC pin 2.
>
> In-kernel irqchip support will once be controlled by the machine
> property 'kernel_irqchip', but this is not yet wired up.
>
>
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support
2011-12-04 13:23 ` Avi Kivity
@ 2011-12-04 13:27 ` Jan Kiszka
2011-12-04 13:28 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 13:27 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 687 bytes --]
On 2011-12-04 14:23, Avi Kivity wrote:
> On 12/03/2011 01:17 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Add the basic infrastructure to active in-kernel irqchip support, inject
>> interrupts into these models, and maintain IRQ routes.
>>
>> Routing is optional and depends on the host arch supporting
>> KVM_CAP_IRQ_ROUTING. When it's not available on x86, we loose the HPET
>
> lose
/me is still looking for a semantic proofreader plugin...
>
>> as we can't route GSI0 to IOAPIC pin 2.
>>
>> In-kernel irqchip support will once be controlled by the machine
>> property 'kernel_irqchip', but this is not yet wired up.
>>
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support
2011-12-04 13:27 ` Jan Kiszka
@ 2011-12-04 13:28 ` Avi Kivity
2011-12-04 13:30 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:28 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 03:27 PM, Jan Kiszka wrote:
> On 2011-12-04 14:23, Avi Kivity wrote:
> > On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Add the basic infrastructure to active in-kernel irqchip support, inject
> >> interrupts into these models, and maintain IRQ routes.
> >>
> >> Routing is optional and depends on the host arch supporting
> >> KVM_CAP_IRQ_ROUTING. When it's not available on x86, we loose the HPET
> >
> > lose
>
> /me is still looking for a semantic proofreader plugin...
>
Well, I have to comment on something. If you don't want spelling
corrections, leave some trailing whitespace.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support
2011-12-04 13:28 ` Avi Kivity
@ 2011-12-04 13:30 ` Jan Kiszka
2011-12-04 13:32 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 13:30 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 769 bytes --]
On 2011-12-04 14:28, Avi Kivity wrote:
> On 12/04/2011 03:27 PM, Jan Kiszka wrote:
>> On 2011-12-04 14:23, Avi Kivity wrote:
>>> On 12/03/2011 01:17 PM, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Add the basic infrastructure to active in-kernel irqchip support, inject
>>>> interrupts into these models, and maintain IRQ routes.
>>>>
>>>> Routing is optional and depends on the host arch supporting
>>>> KVM_CAP_IRQ_ROUTING. When it's not available on x86, we loose the HPET
>>>
>>> lose
>>
>> /me is still looking for a semantic proofreader plugin...
>>
>
> Well, I have to comment on something. If you don't want spelling
> corrections, leave some trailing whitespace.
I could create a messpatch.pl...
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support
2011-12-04 13:30 ` Jan Kiszka
@ 2011-12-04 13:32 ` Avi Kivity
0 siblings, 0 replies; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:32 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 03:30 PM, Jan Kiszka wrote:
> > Well, I have to comment on something. If you don't want spelling
> > corrections, leave some trailing whitespace.
>
> I could create a messpatch.pl...
Ah, and with a --reverse flag we could go through the motions of patch
review without requiring a repost.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* [Qemu-devel] [RFC][PATCH 12/16] kvm: x86: Establish IRQ0 override control
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (10 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 11/16] kvm: Introduce core services for in-kernel irqchip support Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 13/16] kvm: x86: Add user space part for in-kernel APIC Jan Kiszka
` (3 subsequent siblings)
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
KVM is forced to disable the IRQ0 override when we run with in-kernel
irqchip but without IRQ routing support of the kernel. Set the fwcfg
value correspondingly. This aligns us with qemu-kvm.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/pc.c | 3 ++-
kvm-all.c | 5 +++++
kvm-stub.c | 5 +++++
kvm.h | 2 ++
sysemu.h | 1 -
vl.c | 1 -
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 5225d5b..715cc63 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -39,6 +39,7 @@
#include "msi.h"
#include "sysbus.h"
#include "sysemu.h"
+#include "kvm.h"
#include "blockdev.h"
#include "ui/qemu-spice.h"
#include "memory.h"
@@ -609,7 +610,7 @@ static void *bochs_bios_init(void)
fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
acpi_tables_len);
- fw_cfg_add_bytes(fw_cfg, FW_CFG_IRQ0_OVERRIDE, &irq0override, 1);
+ fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
smbios_table = smbios_get_table(&smbios_len);
if (smbios_table)
diff --git a/kvm-all.c b/kvm-all.c
index a85e14f..665455c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1260,6 +1260,11 @@ int kvm_has_gsi_routing(void)
return kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING);
}
+int kvm_allows_irq0_override(void)
+{
+ return !kvm_enabled() || !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
+}
+
void kvm_setup_guest_memory(void *start, size_t size)
{
if (!kvm_has_sync_mmu()) {
diff --git a/kvm-stub.c b/kvm-stub.c
index 06064b9..6c2b06b 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -78,6 +78,11 @@ int kvm_has_many_ioeventfds(void)
return 0;
}
+int kvm_allows_irq0_override(void)
+{
+ return 1;
+}
+
void kvm_setup_guest_memory(void *start, size_t size)
{
}
diff --git a/kvm.h b/kvm.h
index 0d6c453..a3c87af 100644
--- a/kvm.h
+++ b/kvm.h
@@ -53,6 +53,8 @@ int kvm_has_xcrs(void);
int kvm_has_many_ioeventfds(void);
int kvm_has_gsi_routing(void);
+int kvm_allows_irq0_override(void);
+
#ifdef NEED_CPU_H
int kvm_init_vcpu(CPUState *env);
diff --git a/sysemu.h b/sysemu.h
index 22cd720..3bd896e 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -102,7 +102,6 @@ extern int vga_interface_type;
extern int graphic_width;
extern int graphic_height;
extern int graphic_depth;
-extern uint8_t irq0override;
extern DisplayType display_type;
extern const char *keyboard_layout;
extern int win2k_install_hack;
diff --git a/vl.c b/vl.c
index fcce25f..22d02b9 100644
--- a/vl.c
+++ b/vl.c
@@ -218,7 +218,6 @@ int no_reboot = 0;
int no_shutdown = 0;
int cursor_hide = 1;
int graphic_rotate = 0;
-uint8_t irq0override = 1;
const char *watchdog;
QEMUOptionRom option_rom[MAX_OPTION_ROMS];
int nb_option_roms;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 13/16] kvm: x86: Add user space part for in-kernel APIC
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (11 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 12/16] kvm: x86: Establish IRQ0 override control Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-04 13:24 ` Avi Kivity
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259 Jan Kiszka
` (2 subsequent siblings)
15 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
This introduces the alternative APIC model 'kvm-apic' which makes use of
KVM's in-kernel device model. MSI is not yet supported, so we disable
this when the in-kernel model is in use.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.target | 2 +-
hw/kvm/apic.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pc.c | 15 ++++--
kvm.h | 3 +
target-i386/kvm.c | 8 +++
5 files changed, 169 insertions(+), 6 deletions(-)
create mode 100644 hw/kvm/apic.c
diff --git a/Makefile.target b/Makefile.target
index 4cd3c0e..66b42d5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -231,7 +231,7 @@ obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += debugcon.o multiboot.o
obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
# shared objects
diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
new file mode 100644
index 0000000..fe811b6
--- /dev/null
+++ b/hw/kvm/apic.c
@@ -0,0 +1,147 @@
+/*
+ * KVM in-kernel APIC support
+ *
+ * Copyright (c) 2011 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ */
+#include <hw/apic_internal.h>
+#include <kvm.h>
+
+static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
+ int reg_id, uint32_t val)
+{
+ *((uint32_t *)(kapic->regs + (reg_id << 4))) = val;
+}
+
+static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
+ int reg_id)
+{
+ return *((uint32_t *)(kapic->regs + (reg_id << 4)));
+}
+
+int kvm_put_apic(CPUState *env)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, env->apic_state);
+ struct kvm_lapic_state kapic;
+ int i;
+
+ if (s && kvm_enabled() && kvm_irqchip_in_kernel()) {
+ memset(&kapic, 0, sizeof(kapic));
+ kvm_apic_set_reg(&kapic, 0x2, s->id << 24);
+ kvm_apic_set_reg(&kapic, 0x8, s->tpr);
+ kvm_apic_set_reg(&kapic, 0xd, s->log_dest << 24);
+ kvm_apic_set_reg(&kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
+ kvm_apic_set_reg(&kapic, 0xf, s->spurious_vec);
+ for (i = 0; i < 8; i++) {
+ kvm_apic_set_reg(&kapic, 0x10 + i, s->isr[i]);
+ kvm_apic_set_reg(&kapic, 0x18 + i, s->tmr[i]);
+ kvm_apic_set_reg(&kapic, 0x20 + i, s->irr[i]);
+ }
+ kvm_apic_set_reg(&kapic, 0x28, s->esr);
+ kvm_apic_set_reg(&kapic, 0x30, s->icr[0]);
+ kvm_apic_set_reg(&kapic, 0x31, s->icr[1]);
+ for (i = 0; i < APIC_LVT_NB; i++) {
+ kvm_apic_set_reg(&kapic, 0x32 + i, s->lvt[i]);
+ }
+ kvm_apic_set_reg(&kapic, 0x38, s->initial_count);
+ kvm_apic_set_reg(&kapic, 0x3e, s->divide_conf);
+
+ return kvm_vcpu_ioctl(env, KVM_SET_LAPIC, &kapic);
+ }
+
+ return 0;
+}
+
+int kvm_get_apic(CPUState *env)
+{
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, env->apic_state);
+ struct kvm_lapic_state kapic;
+ int ret, i, v;
+
+ if (s && kvm_enabled() && kvm_irqchip_in_kernel()) {
+ ret = kvm_vcpu_ioctl(env, KVM_GET_LAPIC, &kapic);
+ if (ret < 0) {
+ return ret;
+ }
+
+ s->id = kvm_apic_get_reg(&kapic, 0x2) >> 24;
+ s->tpr = kvm_apic_get_reg(&kapic, 0x8);
+ s->arb_id = kvm_apic_get_reg(&kapic, 0x9);
+ s->log_dest = kvm_apic_get_reg(&kapic, 0xd) >> 24;
+ s->dest_mode = kvm_apic_get_reg(&kapic, 0xe) >> 28;
+ s->spurious_vec = kvm_apic_get_reg(&kapic, 0xf);
+ for (i = 0; i < 8; i++) {
+ s->isr[i] = kvm_apic_get_reg(&kapic, 0x10 + i);
+ s->tmr[i] = kvm_apic_get_reg(&kapic, 0x18 + i);
+ s->irr[i] = kvm_apic_get_reg(&kapic, 0x20 + i);
+ }
+ s->esr = kvm_apic_get_reg(&kapic, 0x28);
+ s->icr[0] = kvm_apic_get_reg(&kapic, 0x30);
+ s->icr[1] = kvm_apic_get_reg(&kapic, 0x31);
+ for (i = 0; i < APIC_LVT_NB; i++) {
+ s->lvt[i] = kvm_apic_get_reg(&kapic, 0x32 + i);
+ }
+ s->initial_count = kvm_apic_get_reg(&kapic, 0x38);
+ s->divide_conf = kvm_apic_get_reg(&kapic, 0x3e);
+
+ v = (s->divide_conf & 3) | ((s->divide_conf >> 1) & 4);
+ s->count_shift = (v + 1) & 7;
+
+ s->initial_count_load_time = qemu_get_clock_ns(vm_clock);
+ apic_next_timer(s, s->initial_count_load_time);
+ }
+ return 0;
+}
+
+static void kvm_apic_set_base(APICState *s, uint64_t val)
+{
+ s->apicbase = val;
+}
+
+static void kvm_apic_set_tpr(APICState *s, uint8_t val)
+{
+ s->tpr = (val & 0x0f) << 4;
+}
+
+static int kvm_apic_init(SysBusDevice *dev)
+{
+ APICState *s = FROM_SYSBUS(APICState, dev);
+
+ memory_region_init_reservation(&s->io_memory, "kvm-apic-msi",
+ MSI_SPACE_SIZE);
+
+ if (apic_init_common(s) < 0) {
+ memory_region_destroy(&s->io_memory);
+ return -1;
+ }
+
+ s->set_base = kvm_apic_set_base;
+ s->set_tpr = kvm_apic_set_tpr;
+ return 0;
+}
+
+static SysBusDeviceInfo kvm_apic_info = {
+ .init = kvm_apic_init,
+ .qdev.name = "kvm-apic",
+ .qdev.size = sizeof(APICState),
+ .qdev.vmsd = &vmstate_apic,
+ .qdev.reset = apic_reset,
+ .qdev.no_user = 1,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_UINT8("id", APICState, id, -1),
+ DEFINE_PROP_PTR("cpu_env", APICState, cpu_env),
+ DEFINE_PROP_END_OF_LIST(),
+ }
+};
+
+static void kvm_apic_register_devices(void)
+{
+ sysbus_register_withprop(&kvm_apic_info);
+}
+
+device_init(kvm_apic_register_devices)
diff --git a/hw/pc.c b/hw/pc.c
index 715cc63..0a1e51c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -879,25 +879,30 @@ DeviceState *cpu_get_current_apic(void)
static DeviceState *apic_init(void *env, uint8_t apic_id)
{
DeviceState *dev;
- SysBusDevice *d;
static int apic_mapped;
- dev = qdev_create(NULL, "apic");
+ if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+ dev = qdev_create(NULL, "kvm-apic");
+ } else {
+ dev = qdev_create(NULL, "apic");
+ }
qdev_prop_set_uint8(dev, "id", apic_id);
qdev_prop_set_ptr(dev, "cpu_env", env);
qdev_init_nofail(dev);
- d = sysbus_from_qdev(dev);
/* XXX: mapping more APICs at the same memory location */
if (apic_mapped == 0) {
/* NOTE: the APIC is directly connected to the CPU - it is not
on the global memory bus. */
/* XXX: what if the base changes? */
- sysbus_mmio_map(d, 0, MSI_ADDR_BASE);
+ sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);
apic_mapped = 1;
}
- msi_supported = true;
+ /* KVM does not support MSI yet. */
+ if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
+ msi_supported = true;
+ }
return dev;
}
diff --git a/kvm.h b/kvm.h
index a3c87af..446360a 100644
--- a/kvm.h
+++ b/kvm.h
@@ -134,6 +134,9 @@ int kvm_irqchip_set_irq(KVMState *s, int irq, int level);
void kvm_irqchip_add_route(KVMState *s, int gsi, int irqchip, int pin);
int kvm_irqchip_commit_routes(KVMState *s);
+int kvm_put_apic(CPUState *env);
+int kvm_get_apic(CPUState *env);
+
struct kvm_guest_debug;
struct kvm_debug_exit_arch;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 232c897..537b61f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1435,6 +1435,10 @@ int kvm_arch_put_registers(CPUState *env, int level)
if (ret < 0) {
return ret;
}
+ ret = kvm_put_apic(env);
+ if (ret < 0) {
+ return ret;
+ }
}
ret = kvm_put_vcpu_events(env, level);
if (ret < 0) {
@@ -1482,6 +1486,10 @@ int kvm_arch_get_registers(CPUState *env)
if (ret < 0) {
return ret;
}
+ ret = kvm_get_apic(env);
+ if (ret < 0) {
+ return ret;
+ }
ret = kvm_get_vcpu_events(env);
if (ret < 0) {
return ret;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 13/16] kvm: x86: Add user space part for in-kernel APIC
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 13/16] kvm: x86: Add user space part for in-kernel APIC Jan Kiszka
@ 2011-12-04 13:24 ` Avi Kivity
0 siblings, 0 replies; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:24 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> This introduces the alternative APIC model 'kvm-apic' which makes use of
> KVM's in-kernel device model. MSI is not yet supported, so we disable
> this when the in-kernel model is in use.
>
>
> - dev = qdev_create(NULL, "apic");
> + if (kvm_enabled() && kvm_irqchip_in_kernel()) {
> + dev = qdev_create(NULL, "kvm-apic");
> + } else {
> + dev = qdev_create(NULL, "apic");
> + }
Is there anything that makes those two devices incompatible?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (12 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 13/16] kvm: x86: Add user space part for in-kernel APIC Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-04 13:31 ` Avi Kivity
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 15/16] kvm: x86: Add user space part for in-kernel IOAPIC Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 16/16] kvm: Arm in-kernel irqchip support Jan Kiszka
15 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Introduce the alternative 'kvm-i8259' device model that exploits KVM
in-kernel acceleration.
The PIIX3 initialization code is furthermore extended by KVM specific
IRQ route setup. Moreover, GSI injection differs in KVM mode from the
user space model. As we can dispatch ISA-range IRQs to both IOAPIC and
PIC inside the kernel, we do not need to inject them separately. This is
reflected by a KVM-specific GSI handler.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.target | 2 +-
hw/kvm/i8259.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pc.h | 1 +
hw/pc_piix.c | 50 ++++++++++++++++--
4 files changed, 202 insertions(+), 5 deletions(-)
create mode 100644 hw/kvm/i8259.c
diff --git a/Makefile.target b/Makefile.target
index 66b42d5..850b80f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -231,7 +231,7 @@ obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += debugcon.o multiboot.o
obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
# shared objects
diff --git a/hw/kvm/i8259.c b/hw/kvm/i8259.c
new file mode 100644
index 0000000..9367781
--- /dev/null
+++ b/hw/kvm/i8259.c
@@ -0,0 +1,154 @@
+/*
+ * KVM in-kernel PIC (i8259) support
+ *
+ * Copyright (c) 2011 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ */
+#include <hw/i8259_internal.h>
+#include <hw/apic_internal.h>
+#include <kvm.h>
+
+static void kvm_pic_get(PicState *s)
+{
+ struct kvm_irqchip chip;
+ struct kvm_pic_state *kpic;
+ int ret;
+
+ chip.chip_id = s->master ? KVM_IRQCHIP_PIC_MASTER : KVM_IRQCHIP_PIC_SLAVE;
+ ret = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, &chip);
+ if (ret < 0) {
+ fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+ abort();
+ }
+
+ kpic = &chip.chip.pic;
+
+ s->last_irr = kpic->last_irr;
+ s->irr = kpic->irr;
+ s->imr = kpic->imr;
+ s->isr = kpic->isr;
+ s->priority_add = kpic->priority_add;
+ s->irq_base = kpic->irq_base;
+ s->read_reg_select = kpic->read_reg_select;
+ s->poll = kpic->poll;
+ s->special_mask = kpic->special_mask;
+ s->init_state = kpic->init_state;
+ s->auto_eoi = kpic->auto_eoi;
+ s->rotate_on_auto_eoi = kpic->rotate_on_auto_eoi;
+ s->special_fully_nested_mode = kpic->special_fully_nested_mode;
+ s->init4 = kpic->init4;
+ s->elcr = kpic->elcr;
+ s->elcr_mask = kpic->elcr_mask;
+}
+
+static void kvm_pic_put(PicState *s)
+{
+ struct kvm_irqchip chip;
+ struct kvm_pic_state *kpic;
+ int ret;
+
+ chip.chip_id = s->master ? KVM_IRQCHIP_PIC_MASTER : KVM_IRQCHIP_PIC_SLAVE;
+
+ kpic = &chip.chip.pic;
+
+ kpic->last_irr = s->last_irr;
+ kpic->irr = s->irr;
+ kpic->imr = s->imr;
+ kpic->isr = s->isr;
+ kpic->priority_add = s->priority_add;
+ kpic->irq_base = s->irq_base;
+ kpic->read_reg_select = s->read_reg_select;
+ kpic->poll = s->poll;
+ kpic->special_mask = s->special_mask;
+ kpic->init_state = s->init_state;
+ kpic->auto_eoi = s->auto_eoi;
+ kpic->rotate_on_auto_eoi = s->rotate_on_auto_eoi;
+ kpic->special_fully_nested_mode = s->special_fully_nested_mode;
+ kpic->init4 = s->init4;
+ kpic->elcr = s->elcr;
+ kpic->elcr_mask = s->elcr_mask;
+
+ ret = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, &chip);
+ if (ret < 0) {
+ fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+ abort();
+ }
+}
+
+static void kvm_pic_reset(DeviceState *dev)
+{
+ PicState *s = container_of(dev, PicState, dev.qdev);
+
+ pic_reset_internal(s);
+ s->elcr = 0;
+
+ kvm_pic_put(s);
+}
+
+static void kvm_pic_set_irq(void *opaque, int irq, int level)
+{
+ int delivered;
+
+ delivered = kvm_irqchip_set_irq(kvm_state, irq, level);
+ apic_set_irq_delivered(delivered);
+}
+
+static int kvm_pic_init(ISADevice *dev)
+{
+ PicState *s = DO_UPCAST(PicState, dev, dev);
+
+ memory_region_init_reservation(&s->base_io, "kvm-pic", 2);
+ memory_region_init_reservation(&s->elcr_io, "kvm-elcr", 1);
+
+ pic_init_common(s);
+
+ s->pre_save = kvm_pic_get;
+ s->post_load = kvm_pic_put;
+
+ return 0;
+}
+
+qemu_irq *kvm_i8259_init(void)
+{
+ ISADevice *dev;
+
+ dev = isa_create("kvm-i8259");
+ qdev_prop_set_uint32(&dev->qdev, "iobase", 0x20);
+ qdev_prop_set_uint32(&dev->qdev, "elcr_addr", 0x4d0);
+ qdev_prop_set_bit(&dev->qdev, "master", true);
+ qdev_init_nofail(&dev->qdev);
+
+ dev = isa_create("kvm-i8259");
+ qdev_prop_set_uint32(&dev->qdev, "iobase", 0xa0);
+ qdev_prop_set_uint32(&dev->qdev, "elcr_addr", 0x4d1);
+ qdev_init_nofail(&dev->qdev);
+
+ return qemu_allocate_irqs(kvm_pic_set_irq, NULL, ISA_NUM_IRQS);
+}
+
+static ISADeviceInfo kvm_i8259_info = {
+ .qdev.name = "kvm-i8259",
+ .qdev.size = sizeof(PicState),
+ .qdev.vmsd = &vmstate_pic,
+ .qdev.reset = kvm_pic_reset,
+ .qdev.no_user = 1,
+ .init = kvm_pic_init,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_HEX32("iobase", PicState, iobase, -1),
+ DEFINE_PROP_HEX32("elcr_addr", PicState, elcr_addr, -1),
+ DEFINE_PROP_BIT("master", PicState, master, 0, false),
+ DEFINE_PROP_END_OF_LIST(),
+ },
+};
+
+static void kvm_pic_register(void)
+{
+ isa_qdev_register(&kvm_i8259_info);
+}
+
+device_init(kvm_pic_register)
diff --git a/hw/pc.h b/hw/pc.h
index b8ad9a3..d8e7313 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -63,6 +63,7 @@ bool parallel_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq,
typedef struct PicState PicState;
extern PicState *isa_pic;
qemu_irq *i8259_init(qemu_irq parent_irq);
+qemu_irq *kvm_i8259_init(void);
int pic_read_irq(PicState *s);
int pic_get_output(PicState *s);
void pic_info(Monitor *mon);
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 22997b0..351b032 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -53,6 +53,40 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
+static void kvm_piix3_setup_irq_routing(bool pci_enabled)
+{
+ KVMState *s = kvm_state;
+ int ret, i;
+
+ if (kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) {
+ for (i = 0; i < 8; ++i) {
+ if (i == 2) {
+ continue;
+ }
+ kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_MASTER, i);
+ }
+ for (i = 8; i < 16; ++i) {
+ kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
+ }
+ ret = kvm_irqchip_commit_routes(s);
+ if (ret < 0) {
+ hw_error("KVM IRQ routing setup failed");
+ }
+ }
+}
+
+static void kvm_piix3_gsi_handler(void *opaque, int n, int level)
+{
+ GSIState *s = opaque;
+
+ if (n < ISA_NUM_IRQS) {
+ /* Kernel will forward to both PIC and IOAPIC */
+ qemu_set_irq(s->i8259_irq[n], level);
+ } else {
+ qemu_set_irq(s->ioapic_irq[n], level);
+ }
+}
+
static void ioapic_init(GSIState *gsi_state)
{
DeviceState *dev;
@@ -131,7 +165,13 @@ static void pc_init1(MemoryRegion *system_memory,
}
gsi_state = g_malloc0(sizeof(*gsi_state));
- gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
+ if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+ kvm_piix3_setup_irq_routing(pci_enabled);
+ gsi = qemu_allocate_irqs(kvm_piix3_gsi_handler, gsi_state,
+ GSI_NUM_PINS);
+ } else {
+ gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
+ }
if (pci_enabled) {
pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, gsi,
@@ -151,11 +191,13 @@ static void pc_init1(MemoryRegion *system_memory,
}
isa_bus_irqs(gsi);
- if (!xen_enabled()) {
+ if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+ i8259 = kvm_i8259_init();
+ } else if (xen_enabled()) {
+ i8259 = xen_interrupt_controller_init();
+ } else {
cpu_irq = pc_allocate_cpu_irq();
i8259 = i8259_init(cpu_irq[0]);
- } else {
- i8259 = xen_interrupt_controller_init();
}
for (i = 0; i < ISA_NUM_IRQS; i++) {
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259 Jan Kiszka
@ 2011-12-04 13:31 ` Avi Kivity
2011-12-04 13:42 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:31 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Introduce the alternative 'kvm-i8259' device model that exploits KVM
> in-kernel acceleration.
>
> The PIIX3 initialization code is furthermore extended by KVM specific
> IRQ route setup. Moreover, GSI injection differs in KVM mode from the
> user space model. As we can dispatch ISA-range IRQs to both IOAPIC and
> PIC inside the kernel, we do not need to inject them separately. This is
> reflected by a KVM-specific GSI handler.
>
> +
> +qemu_irq *kvm_i8259_init(void)
> +{
> + ISADevice *dev;
> +
> + dev = isa_create("kvm-i8259");
>
Same issue. Is this a different device, or an different implementation
of the same device?
We're forcing migration from 1.0 to 1.1 to disable in-kernel irqchip on
the target. For qemu itself, that's no issue. But for qemu-kvm, it
will result in loss of performance, or hacks to alias the two back together.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 13:31 ` Avi Kivity
@ 2011-12-04 13:42 ` Jan Kiszka
2011-12-04 13:49 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 13:42 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]
On 2011-12-04 14:31, Avi Kivity wrote:
> On 12/03/2011 01:17 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Introduce the alternative 'kvm-i8259' device model that exploits KVM
>> in-kernel acceleration.
>>
>> The PIIX3 initialization code is furthermore extended by KVM specific
>> IRQ route setup. Moreover, GSI injection differs in KVM mode from the
>> user space model. As we can dispatch ISA-range IRQs to both IOAPIC and
>> PIC inside the kernel, we do not need to inject them separately. This is
>> reflected by a KVM-specific GSI handler.
>>
>> +
>> +qemu_irq *kvm_i8259_init(void)
>> +{
>> + ISADevice *dev;
>> +
>> + dev = isa_create("kvm-i8259");
>>
>
> Same issue. Is this a different device, or an different implementation
> of the same device?
They are theoretically the same from guest perspective (therefore you
can migrate between machines that differ in this).
>
> We're forcing migration from 1.0 to 1.1 to disable in-kernel irqchip on
> the target. For qemu itself, that's no issue. But for qemu-kvm, it
> will result in loss of performance, or hacks to alias the two back together.
We should this happen with qemu-kvm? The vmstates are compatible, thus
you can migration from old qemu-kvm in-kernel devices to the new kvm-*
ones (once they are feature-equivalent). Not sure how much hacks this
may require to qemu-kvm, but I don't think it should make the situation
worse for that tree.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 13:42 ` Jan Kiszka
@ 2011-12-04 13:49 ` Avi Kivity
2011-12-04 13:51 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 13:49 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 03:42 PM, Jan Kiszka wrote:
> On 2011-12-04 14:31, Avi Kivity wrote:
> > On 12/03/2011 01:17 PM, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Introduce the alternative 'kvm-i8259' device model that exploits KVM
> >> in-kernel acceleration.
> >>
> >> The PIIX3 initialization code is furthermore extended by KVM specific
> >> IRQ route setup. Moreover, GSI injection differs in KVM mode from the
> >> user space model. As we can dispatch ISA-range IRQs to both IOAPIC and
> >> PIC inside the kernel, we do not need to inject them separately. This is
> >> reflected by a KVM-specific GSI handler.
> >>
> >> +
> >> +qemu_irq *kvm_i8259_init(void)
> >> +{
> >> + ISADevice *dev;
> >> +
> >> + dev = isa_create("kvm-i8259");
> >>
> >
> > Same issue. Is this a different device, or an different implementation
> > of the same device?
>
> They are theoretically the same from guest perspective (therefore you
> can migrate between machines that differ in this).
But the name becomes part of the save/restore ABI, so you can't.
> >
> > We're forcing migration from 1.0 to 1.1 to disable in-kernel irqchip on
> > the target. For qemu itself, that's no issue. But for qemu-kvm, it
> > will result in loss of performance, or hacks to alias the two back together.
>
> We should this happen with qemu-kvm? The vmstates are compatible, thus
> you can migration from old qemu-kvm in-kernel devices to the new kvm-*
> ones (once they are feature-equivalent). Not sure how much hacks this
> may require to qemu-kvm, but I don't think it should make the situation
> worse for that tree.
They aren't compatible due to the name clash. The hack won't be large
(add an alias for the name), but just one hack is enough to keep the
tree alive for a long while. Better not to add it in the first place.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 13:49 ` Avi Kivity
@ 2011-12-04 13:51 ` Jan Kiszka
2011-12-04 14:04 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 13:51 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
On 2011-12-04 14:49, Avi Kivity wrote:
> On 12/04/2011 03:42 PM, Jan Kiszka wrote:
>> On 2011-12-04 14:31, Avi Kivity wrote:
>>> On 12/03/2011 01:17 PM, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Introduce the alternative 'kvm-i8259' device model that exploits KVM
>>>> in-kernel acceleration.
>>>>
>>>> The PIIX3 initialization code is furthermore extended by KVM specific
>>>> IRQ route setup. Moreover, GSI injection differs in KVM mode from the
>>>> user space model. As we can dispatch ISA-range IRQs to both IOAPIC and
>>>> PIC inside the kernel, we do not need to inject them separately. This is
>>>> reflected by a KVM-specific GSI handler.
>>>>
>>>> +
>>>> +qemu_irq *kvm_i8259_init(void)
>>>> +{
>>>> + ISADevice *dev;
>>>> +
>>>> + dev = isa_create("kvm-i8259");
>>>>
>>>
>>> Same issue. Is this a different device, or an different implementation
>>> of the same device?
>>
>> They are theoretically the same from guest perspective (therefore you
>> can migrate between machines that differ in this).
>
> But the name becomes part of the save/restore ABI, so you can't.
Nope, the vmstate names are identical. That would ruin migration
otherwise. It's just the output of info qtree & co. that changes.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 13:51 ` Jan Kiszka
@ 2011-12-04 14:04 ` Avi Kivity
2011-12-04 14:06 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 14:04 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 03:51 PM, Jan Kiszka wrote:
> >
> > But the name becomes part of the save/restore ABI, so you can't.
>
> Nope, the vmstate names are identical. That would ruin migration
> otherwise. It's just the output of info qtree & co. that changes.
Oh, okay. I still think it's wrong, but now it's just a matter of
taste, and I can live with it.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 14:04 ` Avi Kivity
@ 2011-12-04 14:06 ` Jan Kiszka
2011-12-04 15:12 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 14:06 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 641 bytes --]
On 2011-12-04 15:04, Avi Kivity wrote:
> On 12/04/2011 03:51 PM, Jan Kiszka wrote:
>>>
>>> But the name becomes part of the save/restore ABI, so you can't.
>>
>> Nope, the vmstate names are identical. That would ruin migration
>> otherwise. It's just the output of info qtree & co. that changes.
>
> Oh, okay. I still think it's wrong, but now it's just a matter of
> taste, and I can live with it.
Wrong in what sense?
I think the way of merging kvm support into the user space models in
qemu-kvm is not particularly beautiful. But that's my taste, and
therefore I modeled the upstream proposal differently. :)
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 14:06 ` Jan Kiszka
@ 2011-12-04 15:12 ` Avi Kivity
2011-12-04 15:19 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 15:12 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 04:06 PM, Jan Kiszka wrote:
> On 2011-12-04 15:04, Avi Kivity wrote:
> > On 12/04/2011 03:51 PM, Jan Kiszka wrote:
> >>>
> >>> But the name becomes part of the save/restore ABI, so you can't.
> >>
> >> Nope, the vmstate names are identical. That would ruin migration
> >> otherwise. It's just the output of info qtree & co. that changes.
> >
> > Oh, okay. I still think it's wrong, but now it's just a matter of
> > taste, and I can live with it.
>
> Wrong in what sense?
In the sense that kernel-apic is just an accelerated apic. From the
guest point of view, there's no difference, and that should be reflected
in the device model.
If I'm reading an apic register, either from the guest or via a monitor
debug interface, I shouldn't care whether it's accelerated or not. The
guest part already holds, of course.
> I think the way of merging kvm support into the user space models in
> qemu-kvm is not particularly beautiful. But that's my taste, and
> therefore I modeled the upstream proposal differently. :)
Oh, qemu-kvm was not meant to be an example of engineering elegance,
just minimal changes.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 15:12 ` Avi Kivity
@ 2011-12-04 15:19 ` Jan Kiszka
2011-12-04 16:35 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 15:19 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]
On 2011-12-04 16:12, Avi Kivity wrote:
> On 12/04/2011 04:06 PM, Jan Kiszka wrote:
>> On 2011-12-04 15:04, Avi Kivity wrote:
>>> On 12/04/2011 03:51 PM, Jan Kiszka wrote:
>>>>>
>>>>> But the name becomes part of the save/restore ABI, so you can't.
>>>>
>>>> Nope, the vmstate names are identical. That would ruin migration
>>>> otherwise. It's just the output of info qtree & co. that changes.
>>>
>>> Oh, okay. I still think it's wrong, but now it's just a matter of
>>> taste, and I can live with it.
>>
>> Wrong in what sense?
>
> In the sense that kernel-apic is just an accelerated apic. From the
> guest point of view, there's no difference, and that should be reflected
> in the device model.
That was my goal as well: The guest should not notice the difference,
but the admin on the host side should still be able to tell both
internally fairly different models apart. Plus the code should be
clearly split where there are differences and explicitly shared where
there aren't.
>
> If I'm reading an apic register, either from the guest or via a monitor
> debug interface, I shouldn't care whether it's accelerated or not. The
> guest part already holds, of course.
Specifically for the debug scenario, I'd prefer the clear
differentiation by name as there can always remain subtle differences in
the implementation of kernel vs. user space. Someone debugging the guest
and/or qemu/kvm should remain aware of this.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 15:19 ` Jan Kiszka
@ 2011-12-04 16:35 ` Avi Kivity
2011-12-04 21:31 ` Blue Swirl
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-04 16:35 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 05:19 PM, Jan Kiszka wrote:
> >
> > In the sense that kernel-apic is just an accelerated apic. From the
> > guest point of view, there's no difference, and that should be reflected
> > in the device model.
>
> That was my goal as well: The guest should not notice the difference,
> but the admin on the host side should still be able to tell both
> internally fairly different models apart.
This should be some attribute, not the name.
> Plus the code should be
> clearly split where there are differences and explicitly shared where
> there aren't.
That's a good goal, yes.
>
> >
> > If I'm reading an apic register, either from the guest or via a monitor
> > debug interface, I shouldn't care whether it's accelerated or not. The
> > guest part already holds, of course.
>
> Specifically for the debug scenario, I'd prefer the clear
> differentiation by name as there can always remain subtle differences in
> the implementation of kernel vs. user space. Someone debugging the guest
> and/or qemu/kvm should remain aware of this.
Aware, yes, but the name change is too drastic.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 16:35 ` Avi Kivity
@ 2011-12-04 21:31 ` Blue Swirl
2011-12-04 21:38 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Blue Swirl @ 2011-12-04 21:31 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Jan Kiszka
On Sun, Dec 4, 2011 at 16:35, Avi Kivity <avi@redhat.com> wrote:
> On 12/04/2011 05:19 PM, Jan Kiszka wrote:
>> >
>> > In the sense that kernel-apic is just an accelerated apic. From the
>> > guest point of view, there's no difference, and that should be reflected
>> > in the device model.
>>
>> That was my goal as well: The guest should not notice the difference,
>> but the admin on the host side should still be able to tell both
>> internally fairly different models apart.
>
> This should be some attribute, not the name.
>
>> Plus the code should be
>> clearly split where there are differences and explicitly shared where
>> there aren't.
>
> That's a good goal, yes.
I'd prefer an unified device built from a single source file if
possible. This conflicts with the build-once model though.
>>
>> >
>> > If I'm reading an apic register, either from the guest or via a monitor
>> > debug interface, I shouldn't care whether it's accelerated or not. The
>> > guest part already holds, of course.
>>
>> Specifically for the debug scenario, I'd prefer the clear
>> differentiation by name as there can always remain subtle differences in
>> the implementation of kernel vs. user space. Someone debugging the guest
>> and/or qemu/kvm should remain aware of this.
>
> Aware, yes, but the name change is too drastic.
It should be also possible to migrate from non-KVM device to KVM
version, different names would prevent that for ever.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 21:31 ` Blue Swirl
@ 2011-12-04 21:38 ` Jan Kiszka
2011-12-05 10:01 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-04 21:38 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]
On 2011-12-04 22:31, Blue Swirl wrote:
> On Sun, Dec 4, 2011 at 16:35, Avi Kivity <avi@redhat.com> wrote:
>> On 12/04/2011 05:19 PM, Jan Kiszka wrote:
>>>>
>>>> In the sense that kernel-apic is just an accelerated apic. From the
>>>> guest point of view, there's no difference, and that should be reflected
>>>> in the device model.
>>>
>>> That was my goal as well: The guest should not notice the difference,
>>> but the admin on the host side should still be able to tell both
>>> internally fairly different models apart.
>>
>> This should be some attribute, not the name.
>>
>>> Plus the code should be
>>> clearly split where there are differences and explicitly shared where
>>> there aren't.
>>
>> That's a good goal, yes.
>
> I'd prefer an unified device built from a single source file if
> possible. This conflicts with the build-once model though.
Right, another reason to not do this.
>
>>>
>>>>
>>>> If I'm reading an apic register, either from the guest or via a monitor
>>>> debug interface, I shouldn't care whether it's accelerated or not. The
>>>> guest part already holds, of course.
>>>
>>> Specifically for the debug scenario, I'd prefer the clear
>>> differentiation by name as there can always remain subtle differences in
>>> the implementation of kernel vs. user space. Someone debugging the guest
>>> and/or qemu/kvm should remain aware of this.
>>
>> Aware, yes, but the name change is too drastic.
>
> It should be also possible to migrate from non-KVM device to KVM
> version, different names would prevent that for ever.
It is (theoretically) possible with these patches as the vmstate names
are the same. KVM to TCG migration does not work right now, so I was
only able to test in-kernel <-> user space irqchip model migrations.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-04 21:38 ` Jan Kiszka
@ 2011-12-05 10:01 ` Avi Kivity
2011-12-05 11:37 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-05 10:01 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/04/2011 11:38 PM, Jan Kiszka wrote:
> >
> > It should be also possible to migrate from non-KVM device to KVM
> > version, different names would prevent that for ever.
>
> It is (theoretically) possible with these patches as the vmstate names
> are the same. KVM to TCG migration does not work right now, so I was
> only able to test in-kernel <-> user space irqchip model migrations.
btw, for the next-gen migration protocol, we'd probably be using QOM
paths, not vmstate names; the QOM paths would include the device name?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 10:01 ` Avi Kivity
@ 2011-12-05 11:37 ` Jan Kiszka
2011-12-05 12:36 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-05 11:37 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]
On 2011-12-05 11:01, Avi Kivity wrote:
> On 12/04/2011 11:38 PM, Jan Kiszka wrote:
>>>
>>> It should be also possible to migrate from non-KVM device to KVM
>>> version, different names would prevent that for ever.
>>
>> It is (theoretically) possible with these patches as the vmstate names
>> are the same. KVM to TCG migration does not work right now, so I was
>> only able to test in-kernel <-> user space irqchip model migrations.
>
> btw, for the next-gen migration protocol, we'd probably be using QOM
> paths, not vmstate names; the QOM paths would include the device name?
That would be a very bad idea IMHO. Every refactoring of your device
tree, e.g. to model CPU hotplug and the ICC bus more accurately, would
risk to create a migration crack. At least we would need some stable
naming and/or alias concept then.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 11:37 ` Jan Kiszka
@ 2011-12-05 12:36 ` Avi Kivity
2011-12-05 12:47 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-05 12:36 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/05/2011 01:37 PM, Jan Kiszka wrote:
> On 2011-12-05 11:01, Avi Kivity wrote:
> > On 12/04/2011 11:38 PM, Jan Kiszka wrote:
> >>>
> >>> It should be also possible to migrate from non-KVM device to KVM
> >>> version, different names would prevent that for ever.
> >>
> >> It is (theoretically) possible with these patches as the vmstate names
> >> are the same. KVM to TCG migration does not work right now, so I was
> >> only able to test in-kernel <-> user space irqchip model migrations.
> >
> > btw, for the next-gen migration protocol, we'd probably be using QOM
> > paths, not vmstate names; the QOM paths would include the device name?
>
> That would be a very bad idea IMHO. Every refactoring of your device
> tree, e.g. to model CPU hotplug and the ICC bus more accurately, would
> risk to create a migration crack.
At some point, something has to be stable. We can't have an infinite
number of layers giving names to things. I propose we have just one layer.
> At least we would need some stable
> naming and/or alias concept then.
We should be able to transform a path to backward compatible names,
yes. But if something has an unstable name, let's omit it in the first
place.
(the memory API added unstable names, hopefully the QOM can take over
the stable ones and we'll have a good way to denote the unstable ones).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 12:36 ` Avi Kivity
@ 2011-12-05 12:47 ` Jan Kiszka
2011-12-05 13:14 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-05 12:47 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 2011-12-05 13:36, Avi Kivity wrote:
> On 12/05/2011 01:37 PM, Jan Kiszka wrote:
>> On 2011-12-05 11:01, Avi Kivity wrote:
>>> On 12/04/2011 11:38 PM, Jan Kiszka wrote:
>>>>>
>>>>> It should be also possible to migrate from non-KVM device to KVM
>>>>> version, different names would prevent that for ever.
>>>>
>>>> It is (theoretically) possible with these patches as the vmstate names
>>>> are the same. KVM to TCG migration does not work right now, so I was
>>>> only able to test in-kernel <-> user space irqchip model migrations.
>>>
>>> btw, for the next-gen migration protocol, we'd probably be using QOM
>>> paths, not vmstate names; the QOM paths would include the device name?
>>
>> That would be a very bad idea IMHO. Every refactoring of your device
>> tree, e.g. to model CPU hotplug and the ICC bus more accurately, would
>> risk to create a migration crack.
>
> At some point, something has to be stable. We can't have an infinite
> number of layers giving names to things. I propose we have just one layer.
>
>> At least we would need some stable
>> naming and/or alias concept then.
>
> We should be able to transform a path to backward compatible names,
> yes. But if something has an unstable name, let's omit it in the first
> place.
>
> (the memory API added unstable names, hopefully the QOM can take over
> the stable ones and we'll have a good way to denote the unstable ones).
>
OK, maybe - or likely - we should make those device models have the same
names in QOM once instantiated. But I'm still convinced they should
remain separated models in contrast to a single model with a property.
The kvm ioapic, e.g., requires an additional property (gsi_base) that is
meaningless for user space devices. And its interrupts have to be
wired&configured differently at board model level. So, from the QEMU
POV, it is a very different device. Just the guest does not notice.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 12:47 ` Jan Kiszka
@ 2011-12-05 13:14 ` Avi Kivity
2011-12-05 13:29 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-05 13:14 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/05/2011 02:47 PM, Jan Kiszka wrote:
> >
> > (the memory API added unstable names, hopefully the QOM can take over
> > the stable ones and we'll have a good way to denote the unstable ones).
> >
>
> OK, maybe - or likely - we should make those device models have the same
> names in QOM once instantiated. But I'm still convinced they should
> remain separated models in contrast to a single model with a property.
What do you mean by separate models? You share all the code you can,
and don't share the code you can't. To me, single model == single name.
> The kvm ioapic, e.g., requires an additional property (gsi_base) that is
> meaningless for user space devices. And its interrupts have to be
> wired&configured differently at board model level. So, from the QEMU
> POV, it is a very different device. Just the guest does not notice.
It's like qcow2 and raw/native IO are wire differently, or virtio-net
and vhost-net. But it's the same IDE device or virtio NIC.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 13:14 ` Avi Kivity
@ 2011-12-05 13:29 ` Jan Kiszka
2011-12-05 13:36 ` Avi Kivity
0 siblings, 1 reply; 54+ messages in thread
From: Jan Kiszka @ 2011-12-05 13:29 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 2011-12-05 14:14, Avi Kivity wrote:
> On 12/05/2011 02:47 PM, Jan Kiszka wrote:
>>>
>>> (the memory API added unstable names, hopefully the QOM can take over
>>> the stable ones and we'll have a good way to denote the unstable ones).
>>>
>>
>> OK, maybe - or likely - we should make those device models have the same
>> names in QOM once instantiated. But I'm still convinced they should
>> remain separated models in contrast to a single model with a property.
>
> What do you mean by separate models? You share all the code you can,
> and don't share the code you can't. To me, single model == single name.
But different configuration.
>
>> The kvm ioapic, e.g., requires an additional property (gsi_base) that is
>> meaningless for user space devices. And its interrupts have to be
>> wired&configured differently at board model level. So, from the QEMU
>> POV, it is a very different device. Just the guest does not notice.
>
> It's like qcow2 and raw/native IO are wire differently, or virtio-net
> and vhost-net. But it's the same IDE device or virtio NIC.
That would mean introducing a backend/frontend concept for irqchips.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 13:29 ` Jan Kiszka
@ 2011-12-05 13:36 ` Avi Kivity
2011-12-05 13:55 ` Jan Kiszka
0 siblings, 1 reply; 54+ messages in thread
From: Avi Kivity @ 2011-12-05 13:36 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Michael S. Tsirkin, Marcelo Tosatti,
qemu-devel, Blue Swirl
On 12/05/2011 03:29 PM, Jan Kiszka wrote:
> On 2011-12-05 14:14, Avi Kivity wrote:
> > On 12/05/2011 02:47 PM, Jan Kiszka wrote:
> >>>
> >>> (the memory API added unstable names, hopefully the QOM can take over
> >>> the stable ones and we'll have a good way to denote the unstable ones).
> >>>
> >>
> >> OK, maybe - or likely - we should make those device models have the same
> >> names in QOM once instantiated. But I'm still convinced they should
> >> remain separated models in contrast to a single model with a property.
> >
> > What do you mean by separate models? You share all the code you can,
> > and don't share the code you can't. To me, single model == single name.
>
> But different configuration.
Right, just like IDE with different backends.
> >
> >> The kvm ioapic, e.g., requires an additional property (gsi_base) that is
> >> meaningless for user space devices. And its interrupts have to be
> >> wired&configured differently at board model level. So, from the QEMU
> >> POV, it is a very different device. Just the guest does not notice.
> >
> > It's like qcow2 and raw/native IO are wire differently, or virtio-net
> > and vhost-net. But it's the same IDE device or virtio NIC.
>
> That would mean introducing a backend/frontend concept for irqchips.
We could do it, have one ioapic model with ioapic_ops->eoi_broadcast().
Most of the interfaces already dispatch dynamically (qdev gpio/irq) so
there wouldn't be much more there.
To me, how it's actually implemented is not important. What is
important is that save/restore, the monitor, and the guest don't notice
any changes.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259
2011-12-05 13:36 ` Avi Kivity
@ 2011-12-05 13:55 ` Jan Kiszka
0 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-05 13:55 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, kvm@vger.kernel.org, Michael S. Tsirkin,
Marcelo Tosatti, qemu-devel, Blue Swirl
On 2011-12-05 14:36, Avi Kivity wrote:
> On 12/05/2011 03:29 PM, Jan Kiszka wrote:
>> On 2011-12-05 14:14, Avi Kivity wrote:
>>> On 12/05/2011 02:47 PM, Jan Kiszka wrote:
>>>>>
>>>>> (the memory API added unstable names, hopefully the QOM can take over
>>>>> the stable ones and we'll have a good way to denote the unstable ones).
>>>>>
>>>>
>>>> OK, maybe - or likely - we should make those device models have the same
>>>> names in QOM once instantiated. But I'm still convinced they should
>>>> remain separated models in contrast to a single model with a property.
>>>
>>> What do you mean by separate models? You share all the code you can,
>>> and don't share the code you can't. To me, single model == single name.
>>
>> But different configuration.
>
> Right, just like IDE with different backends.
Except that there is a comparably large infrastructure to manage those
backends.
>
>>>
>>>> The kvm ioapic, e.g., requires an additional property (gsi_base) that is
>>>> meaningless for user space devices. And its interrupts have to be
>>>> wired&configured differently at board model level. So, from the QEMU
>>>> POV, it is a very different device. Just the guest does not notice.
>>>
>>> It's like qcow2 and raw/native IO are wire differently, or virtio-net
>>> and vhost-net. But it's the same IDE device or virtio NIC.
>>
>> That would mean introducing a backend/frontend concept for irqchips.
>
> We could do it, have one ioapic model with ioapic_ops->eoi_broadcast().
> Most of the interfaces already dispatch dynamically (qdev gpio/irq) so
> there wouldn't be much more there.
The problem is configuration. Just by setting ioapic.backend=xxx, we
cannot pass down parameters that are backend-specific. We could ignore
this issue and make all specific parameters visible via the frontend.
Would be slightly ugly.
>
> To me, how it's actually implemented is not important. What is
> important is that save/restore, the monitor, and the guest don't notice
> any changes.
I widely agree, except that differentiation (or backend awareness) has
to be preserved in the monitor.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 54+ messages in thread
* [Qemu-devel] [RFC][PATCH 15/16] kvm: x86: Add user space part for in-kernel IOAPIC
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (13 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 14/16] kvm: x86: Add user space part for in-kernel i8259 Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 16/16] kvm: Arm in-kernel irqchip support Jan Kiszka
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
This introduces the KVM-accelerated IOAPIC model 'kvm-ioapic' and
extends the IRQ routing setup by the 0->2 redirection when needed.
The kvm-ioapic model has a property that allows to define its GSI base
for injecting interrupts into the kernel model. This will allow to
disentangle PIC and IOAPIC pins for chipsets that support more
sophisticated IRQ routes than the PIIX3. So far the base is kept at 0,
i.e. PIC and IOAPIC share pins 0..15.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.target | 2 +-
hw/ioapic_internal.h | 1 +
hw/kvm/ioapic.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pc_piix.c | 15 ++++++-
4 files changed, 136 insertions(+), 2 deletions(-)
create mode 100644 hw/kvm/ioapic.c
diff --git a/Makefile.target b/Makefile.target
index 850b80f..2f3407b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -231,7 +231,7 @@ obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += debugcon.o multiboot.o
obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
# shared objects
diff --git a/hw/ioapic_internal.h b/hw/ioapic_internal.h
index bda3608..7d5f735 100644
--- a/hw/ioapic_internal.h
+++ b/hw/ioapic_internal.h
@@ -83,6 +83,7 @@ struct IOAPICState {
void (*pre_save)(IOAPICState *s);
void (*post_load)(IOAPICState *s);
+ uint32_t kvm_gsi_base;
};
extern const VMStateDescription vmstate_ioapic;
diff --git a/hw/kvm/ioapic.c b/hw/kvm/ioapic.c
new file mode 100644
index 0000000..680229b
--- /dev/null
+++ b/hw/kvm/ioapic.c
@@ -0,0 +1,120 @@
+/*
+ * KVM in-kernel IOPIC support
+ *
+ * Copyright (c) 2011 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <hw/pc.h>
+#include <hw/ioapic_internal.h>
+#include <hw/apic_internal.h>
+#include <kvm.h>
+
+static void kvm_ioapic_get(IOAPICState *s)
+{
+ struct kvm_irqchip chip;
+ struct kvm_ioapic_state *kioapic;
+ int ret, i;
+
+ chip.chip_id = KVM_IRQCHIP_IOAPIC;
+ ret = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, &chip);
+ if (ret < 0) {
+ fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+ abort();
+ }
+
+ kioapic = &chip.chip.ioapic;
+
+ s->id = kioapic->id;
+ s->ioregsel = kioapic->ioregsel;
+ s->irr = kioapic->irr;
+ for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+ s->ioredtbl[i] = kioapic->redirtbl[i].bits;
+ }
+}
+
+static void kvm_ioapic_put(IOAPICState *s)
+{
+ struct kvm_irqchip chip;
+ struct kvm_ioapic_state *kioapic;
+ int ret, i;
+
+ chip.chip_id = KVM_IRQCHIP_IOAPIC;
+ kioapic = &chip.chip.ioapic;
+
+ kioapic->id = s->id;
+ kioapic->ioregsel = s->ioregsel;
+ kioapic->base_address = s->busdev.mmio[0].addr;
+ kioapic->irr = s->irr;
+ for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+ kioapic->redirtbl[i].bits = s->ioredtbl[i];
+ }
+
+ ret = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, &chip);
+ if (ret < 0) {
+ fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+ abort();
+ }
+}
+
+static void kvm_ioapic_reset(DeviceState *d)
+{
+ IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
+
+ ioapic_reset_internal(s);
+
+ kvm_ioapic_put(s);
+}
+
+static void kvm_ioapic_set_irq(void *opaque, int irq, int level)
+{
+ IOAPICState *s = opaque;
+ int delivered;
+
+ delivered = kvm_irqchip_set_irq(kvm_state, s->kvm_gsi_base + irq, level);
+ apic_set_irq_delivered(delivered);
+}
+
+static int kvm_ioapic_init(SysBusDevice *dev)
+{
+ IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
+
+ memory_region_init_reservation(&s->io_memory, "kvm-ioapic", 0x1000);
+
+ if (ioapic_init_common(s) < 0) {
+ memory_region_destroy(&s->io_memory);
+ return -1;
+ }
+
+ s->pre_save = kvm_ioapic_get;
+ s->post_load = kvm_ioapic_put;
+
+ qdev_init_gpio_in(&dev->qdev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+
+ return 0;
+}
+
+static SysBusDeviceInfo kvm_ioapic_info = {
+ .init = kvm_ioapic_init,
+ .qdev.name = "kvm-ioapic",
+ .qdev.size = sizeof(IOAPICState),
+ .qdev.vmsd = &vmstate_ioapic,
+ .qdev.reset = kvm_ioapic_reset,
+ .qdev.no_user = 1,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_UINT32("gsi_base", IOAPICState, kvm_gsi_base, 0),
+ DEFINE_PROP_END_OF_LIST(),
+ }
+};
+
+static void kvm_ioapic_register_devices(void)
+{
+ sysbus_register_withprop(&kvm_ioapic_info);
+}
+
+device_init(kvm_ioapic_register_devices)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 351b032..624aecd 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -68,6 +68,15 @@ static void kvm_piix3_setup_irq_routing(bool pci_enabled)
for (i = 8; i < 16; ++i) {
kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
}
+ if (pci_enabled) {
+ for (i = 0; i < 24; ++i) {
+ if (i == 0) {
+ kvm_irqchip_add_route(s, i, KVM_IRQCHIP_IOAPIC, 2);
+ } else if (i != 2) {
+ kvm_irqchip_add_route(s, i, KVM_IRQCHIP_IOAPIC, i);
+ }
+ }
+ }
ret = kvm_irqchip_commit_routes(s);
if (ret < 0) {
hw_error("KVM IRQ routing setup failed");
@@ -93,7 +102,11 @@ static void ioapic_init(GSIState *gsi_state)
SysBusDevice *d;
unsigned int i;
- dev = qdev_create(NULL, "ioapic");
+ if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+ dev = qdev_create(NULL, "kvm-ioapic");
+ } else {
+ dev = qdev_create(NULL, "ioapic");
+ }
qdev_init_nofail(dev);
d = sysbus_from_qdev(dev);
sysbus_mmio_map(d, 0, 0xfec00000);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread* [Qemu-devel] [RFC][PATCH 16/16] kvm: Arm in-kernel irqchip support
2011-12-03 11:17 [Qemu-devel] [RFC][PATCH 00/16] uq/master: Introduce basic irqchip support Jan Kiszka
` (14 preceding siblings ...)
2011-12-03 11:17 ` [Qemu-devel] [RFC][PATCH 15/16] kvm: x86: Add user space part for in-kernel IOAPIC Jan Kiszka
@ 2011-12-03 11:17 ` Jan Kiszka
15 siblings, 0 replies; 54+ messages in thread
From: Jan Kiszka @ 2011-12-03 11:17 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti
Cc: Blue Swirl, Anthony Liguori, qemu-devel, kvm, Michael S. Tsirkin
From: Jan Kiszka <jan.kiszka@siemens.com>
Make the basic in-kernel irqchip support selectable via
-machine ...,kernel_irqchip=on. Leave it off by default until it can
fully replace user space models.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu-config.c | 4 ++++
qemu-options.hx | 5 ++++-
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 90b6b3e..fc25115 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -483,6 +483,10 @@ static QemuOptsList qemu_machine_opts = {
.name = "accel",
.type = QEMU_OPT_STRING,
.help = "accelerator list",
+ }, {
+ .name = "kernel_irqchip",
+ .type = QEMU_OPT_BOOL,
+ .help = "use KVM in-kernel irqchip",
},
{ /* End of list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 5d2a776..e10186b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -31,7 +31,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
"-machine [type=]name[,prop[=value][,...]]\n"
" selects emulated machine (-machine ? for list)\n"
" property accel=accel1[:accel2[:...]] selects accelerator\n"
- " supported accelerators are kvm, xen, tcg (default: tcg)\n",
+ " supported accelerators are kvm, xen, tcg (default: tcg)\n"
+ " kernel_irqchip=on|off controls accelerated irqchip support\n",
QEMU_ARCH_ALL)
STEXI
@item -machine [type=]@var{name}[,prop=@var{value}[,...]]
@@ -44,6 +45,8 @@ This is used to enable an accelerator. Depending on the target architecture,
kvm, xen, or tcg can be available. By default, tcg is used. If there is more
than one accelerator specified, the next one is used if the previous one fails
to initialize.
+@item kernel_irqchip=on|off
+Enables in-kernel irqchip support for the chosen accelerator when available.
@end table
ETEXI
--
1.7.3.4
^ permalink raw reply related [flat|nested] 54+ messages in thread