Re: [PATCH 3/5] x86: move more functions to X86MachineState and x86.c

[Paolo Bonzini]

On 12/12/19 16:59, Sergio Lopez wrote:
> 
> Paolo Bonzini <address@hidden> writes:
> 
>> These are needed by microvm too, so move them outside of PC-specific files.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>  hw/i386/pc.c          |  99 
>> -------------------------------------------------
>>  hw/i386/x86.c         | 101 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/i386/pc.h  |  12 ------
>>  include/hw/i386/x86.h |  13 +++++++
>>  4 files changed, 114 insertions(+), 111 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 3d2c5d8..308248d 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -342,17 +342,6 @@ GlobalProperty pc_compat_1_4[] = {
>>  };
>>  const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);
>>  
>> -void gsi_handler(void *opaque, int n, int level)
>> -{
>> -    GSIState *s = opaque;
>> -
>> -    DPRINTF("pc: %s GSI %d\n", level ? "raising" : "lowering", n);
>> -    if (n < ISA_NUM_IRQS) {
>> -        qemu_set_irq(s->i8259_irq[n], level);
>> -    }
>> -    qemu_set_irq(s->ioapic_irq[n], level);
>> -}
>> -
>>  GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
>>  {
>>      GSIState *s;
>> @@ -392,55 +381,6 @@ static uint64_t ioportF0_read(void *opaque, hwaddr 
>> addr, unsigned size)
>>      return 0xffffffffffffffffULL;
>>  }
>>  
>> -/* TSC handling */
>> -uint64_t cpu_get_tsc(CPUX86State *env)
>> -{
>> -    return cpu_get_ticks();
>> -}
>> -
>> -/* IRQ handling */
>> -int cpu_get_pic_interrupt(CPUX86State *env)
>> -{
>> -    X86CPU *cpu = env_archcpu(env);
>> -    int intno;
>> -
>> -    if (!kvm_irqchip_in_kernel()) {
>> -        intno = apic_get_interrupt(cpu->apic_state);
>> -        if (intno >= 0) {
>> -            return intno;
>> -        }
>> -        /* read the irq from the PIC */
>> -        if (!apic_accept_pic_intr(cpu->apic_state)) {
>> -            return -1;
>> -        }
>> -    }
>> -
>> -    intno = pic_read_irq(isa_pic);
>> -    return intno;
>> -}
>> -
>> -static void pic_irq_request(void *opaque, int irq, int level)
>> -{
>> -    CPUState *cs = first_cpu;
>> -    X86CPU *cpu = X86_CPU(cs);
>> -
>> -    DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
>> -    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
>> -        CPU_FOREACH(cs) {
>> -            cpu = X86_CPU(cs);
>> -            if (apic_accept_pic_intr(cpu->apic_state)) {
>> -                apic_deliver_pic_intr(cpu->apic_state, level);
>> -            }
>> -        }
>> -    } else {
>> -        if (level) {
>> -            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>> -        } else {
>> -            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>> -        }
>> -    }
>> -}
>> -
>>  /* PC cmos mappings */
>>  
>>  #define REG_EQUIPMENT_BYTE          0x14
>> @@ -884,16 +824,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
>>      nb_ne2k++;
>>  }
>>  
>> -DeviceState *cpu_get_current_apic(void)
>> -{
>> -    if (current_cpu) {
>> -        X86CPU *cpu = X86_CPU(current_cpu);
>> -        return cpu->apic_state;
>> -    } else {
>> -        return NULL;
>> -    }
>> -}
>> -
>>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>>  {
>>      X86CPU *cpu = opaque;
>> @@ -1289,11 +1219,6 @@ uint64_t pc_pci_hole64_start(void)
>>      return ROUND_UP(hole64_start, 1 * GiB);
>>  }
>>  
>> -qemu_irq pc_allocate_cpu_irq(void)
>> -{
>> -    return qemu_allocate_irq(pic_irq_request, NULL, 0);
>> -}
>> -
>>  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
>>  {
>>      DeviceState *dev = NULL;
>> @@ -1480,30 +1405,6 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq 
>> *i8259_irqs)
>>      g_free(i8259);
>>  }
>>  
>> -void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
>> -{
>> -    DeviceState *dev;
>> -    SysBusDevice *d;
>> -    unsigned int i;
>> -
>> -    if (kvm_ioapic_in_kernel()) {
>> -        dev = qdev_create(NULL, TYPE_KVM_IOAPIC);
>> -    } else {
>> -        dev = qdev_create(NULL, TYPE_IOAPIC);
>> -    }
>> -    if (parent_name) {
>> -        object_property_add_child(object_resolve_path(parent_name, NULL),
>> -                                  "ioapic", OBJECT(dev), NULL);
>> -    }
>> -    qdev_init_nofail(dev);
>> -    d = SYS_BUS_DEVICE(dev);
>> -    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
>> -
>> -    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
>> -        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
>> -    }
>> -}
>> -
>>  static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState 
>> *dev,
>>                                 Error **errp)
>>  {
>> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> index 394edc2..028bbf4 100644
>> --- a/hw/i386/x86.c
>> +++ b/hw/i386/x86.c
>> @@ -41,11 +41,15 @@
>>  #include "hw/i386/fw_cfg.h"
>>  
>>  #include "hw/acpi/cpu_hotplug.h"
>> +#include "hw/i386/pc.h"
> 
> Including pc.h from x86.h looks a bit weird to me. I think it'd be nicer
> moving the i8259 declarations to a separate header, and the all the
> GSI-related stuff to x86.h. I have a couple patches doing that, in case
> they can save you some time.

Hmm I wonder if the include is actually needed.  Thanks for pointing it out.

Paolo

signature.asc
Description: OpenPGP digital signature