| Message ID | 20191015162705.28087-3-philmd@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/i386/pc: Split PIIX3 southbridge from i440FX northbridge | expand |
On Tuesday, October 15, 2019, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Move the KVM-related call to "sysemu/kvm.h". > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/i386/pc.h | 1 - > include/sysemu/kvm.h | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > Is there any other similar case in our code base? A. > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 6df4f4b6fb..09e74e7764 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -158,7 +158,6 @@ typedef struct PCMachineClass { > > extern DeviceState *isa_pic; > qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq); > -qemu_irq *kvm_i8259_init(ISABus *bus); > int pic_read_irq(DeviceState *d); > int pic_get_output(DeviceState *d); > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 9d143282bc..da8aa9f5a8 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, > qemu_irq irq, int gsi); > void kvm_pc_gsi_handler(void *opaque, int n, int level); > void kvm_pc_setup_irq_routing(bool pci_enabled); > void kvm_init_irq_routing(KVMState *s); > +qemu_irq *kvm_i8259_init(ISABus *bus); > > /** > * kvm_arch_irqchip_create: > -- > 2.21.0 > > >
On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote: > Move the KVM-related call to "sysemu/kvm.h". > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/i386/pc.h | 1 - > include/sysemu/kvm.h | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 6df4f4b6fb..09e74e7764 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -158,7 +158,6 @@ typedef struct PCMachineClass { > > extern DeviceState *isa_pic; > qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq); > -qemu_irq *kvm_i8259_init(ISABus *bus); > int pic_read_irq(DeviceState *d); > int pic_get_output(DeviceState *d); > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 9d143282bc..da8aa9f5a8 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi); > void kvm_pc_gsi_handler(void *opaque, int n, int level); > void kvm_pc_setup_irq_routing(bool pci_enabled); > void kvm_init_irq_routing(KVMState *s); > +qemu_irq *kvm_i8259_init(ISABus *bus); Why? The function is defined in hw/i386/kvm/ - so moving its prototype to a generic header sounds wrong to me. Thomas
On 10/17/19 4:57 PM, Aleksandar Markovic wrote: > > > On Tuesday, October 15, 2019, Philippe Mathieu-Daudé <philmd@redhat.com > <mailto:philmd@redhat.com>> wrote: > > Move the KVM-related call to "sysemu/kvm.h". Maybe s/call/function declaration/ > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com > <mailto:philmd@redhat.com>> > --- > include/hw/i386/pc.h | 1 - > include/sysemu/kvm.h | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > > Is there any other similar case in our code base? These look appropriate: include/hw/ppc/openpic_kvm.h:5:int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs); include/hw/timer/i8254.h:67:static inline ISADevice *kvm_pit_init(ISABus *bus, int base) hw/intc/vgic_common.h:25: * kvm_arm_gic_set_irq - Send an IRQ to the in-kernel vGIC hw/intc/vgic_common.h:33:void kvm_arm_gic_set_irq(uint32_t num_irq, int irq, int level); although kvm_pit_init() is probably borderline. > > A. > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 6df4f4b6fb..09e74e7764 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -158,7 +158,6 @@ typedef struct PCMachineClass { > > extern DeviceState *isa_pic; > qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq); > -qemu_irq *kvm_i8259_init(ISABus *bus); > int pic_read_irq(DeviceState *d); > int pic_get_output(DeviceState *d); > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 9d143282bc..da8aa9f5a8 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, > qemu_irq irq, int gsi); > void kvm_pc_gsi_handler(void *opaque, int n, int level); > void kvm_pc_setup_irq_routing(bool pci_enabled); > void kvm_init_irq_routing(KVMState *s); > +qemu_irq *kvm_i8259_init(ISABus *bus); > > /** > * kvm_arch_irqchip_create: > -- > 2.21.0 > >
On 10/17/19 5:04 PM, Thomas Huth wrote: > On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote: >> Move the KVM-related call to "sysemu/kvm.h". >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/hw/i386/pc.h | 1 - >> include/sysemu/kvm.h | 1 + >> 2 files changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 6df4f4b6fb..09e74e7764 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -158,7 +158,6 @@ typedef struct PCMachineClass { >> >> extern DeviceState *isa_pic; >> qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq); >> -qemu_irq *kvm_i8259_init(ISABus *bus); >> int pic_read_irq(DeviceState *d); >> int pic_get_output(DeviceState *d); >> >> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h >> index 9d143282bc..da8aa9f5a8 100644 >> --- a/include/sysemu/kvm.h >> +++ b/include/sysemu/kvm.h >> @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi); >> void kvm_pc_gsi_handler(void *opaque, int n, int level); >> void kvm_pc_setup_irq_routing(bool pci_enabled); >> void kvm_init_irq_routing(KVMState *s); >> +qemu_irq *kvm_i8259_init(ISABus *bus); > > Why? The function is defined in hw/i386/kvm/ - so moving its prototype > to a generic header sounds wrong to me. This function is declared when compiling without KVM, and is available on the Alpha/HPPA/MIPS which don't have it. You'd rather move the kvm_pc_* declarations to hw/i386/kvm/?
On 17/10/2019 17.31, Philippe Mathieu-Daudé wrote: > On 10/17/19 5:04 PM, Thomas Huth wrote: >> On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote: >>> Move the KVM-related call to "sysemu/kvm.h". >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> --- >>> include/hw/i386/pc.h | 1 - >>> include/sysemu/kvm.h | 1 + >>> 2 files changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >>> index 6df4f4b6fb..09e74e7764 100644 >>> --- a/include/hw/i386/pc.h >>> +++ b/include/hw/i386/pc.h >>> @@ -158,7 +158,6 @@ typedef struct PCMachineClass { >>> extern DeviceState *isa_pic; >>> qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq); >>> -qemu_irq *kvm_i8259_init(ISABus *bus); >>> int pic_read_irq(DeviceState *d); >>> int pic_get_output(DeviceState *d); >>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h >>> index 9d143282bc..da8aa9f5a8 100644 >>> --- a/include/sysemu/kvm.h >>> +++ b/include/sysemu/kvm.h >>> @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, >>> qemu_irq irq, int gsi); >>> void kvm_pc_gsi_handler(void *opaque, int n, int level); >>> void kvm_pc_setup_irq_routing(bool pci_enabled); >>> void kvm_init_irq_routing(KVMState *s); >>> +qemu_irq *kvm_i8259_init(ISABus *bus); >> >> Why? The function is defined in hw/i386/kvm/ - so moving its prototype >> to a generic header sounds wrong to me. > > This function is declared when compiling without KVM, and is available > on the Alpha/HPPA/MIPS which don't have it. Sorry, I failed to parse your last sentence. It's only used by hw/i386 code as far as I can see. > You'd rather move the kvm_pc_* declarations to hw/i386/kvm/? Maybe, but that's certainly something for a different patch series. This series here should focus on what you've mentioned in the cover letter, I think. It's already big enough. Thomas
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 6df4f4b6fb..09e74e7764 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -158,7 +158,6 @@ typedef struct PCMachineClass { extern DeviceState *isa_pic; qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq); -qemu_irq *kvm_i8259_init(ISABus *bus); int pic_read_irq(DeviceState *d); int pic_get_output(DeviceState *d); diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 9d143282bc..da8aa9f5a8 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi); void kvm_pc_gsi_handler(void *opaque, int n, int level); void kvm_pc_setup_irq_routing(bool pci_enabled); void kvm_init_irq_routing(KVMState *s); +qemu_irq *kvm_i8259_init(ISABus *bus); /** * kvm_arch_irqchip_create:
Move the KVM-related call to "sysemu/kvm.h". Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/i386/pc.h | 1 - include/sysemu/kvm.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)