Re: [PATCH v5 04/10] hw/i386: split PCMachineState deriving X86MachineState from it

[Sergio Lopez <slp@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 6047 bytes --]


Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Hi Sergio,
>
> On 10/2/19 1:30 PM, Sergio Lopez wrote:
>> Split up PCMachineState and PCMachineClass and derive X86MachineState
>> and X86MachineClass from them. This allows sharing code with non-PC
>> x86 machine types.
>>
>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>> ---
>>   hw/acpi/cpu_hotplug.c |  10 +--
>>   hw/i386/acpi-build.c  |  29 ++++---
>>   hw/i386/amd_iommu.c   |   3 +-
>>   hw/i386/intel_iommu.c |   3 +-
>>   hw/i386/pc.c          | 178 ++++++++++++++----------------------------
>>   hw/i386/pc_piix.c     |  43 +++++-----
>>   hw/i386/pc_q35.c      |  35 +++++----
>>   hw/i386/x86.c         | 139 +++++++++++++++++++++++++++++----
>>   hw/intc/ioapic.c      |   2 +-
>>   include/hw/i386/pc.h  |  27 +------
>>   include/hw/i386/x86.h |  56 ++++++++++++-
>>   11 files changed, 308 insertions(+), 217 deletions(-)
>>
> [...]
>> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
>> index bc1b594a93..5de2f91845 100644
>> --- a/include/hw/i386/x86.h
>> +++ b/include/hw/i386/x86.h
>> @@ -17,7 +17,61 @@
>>   #ifndef HW_I386_X86_H
>>   #define HW_I386_X86_H
>>   +#include "qemu-common.h"
>> +#include "exec/hwaddr.h"
>> +#include "qemu/notify.h"
>> +
>>   #include "hw/boards.h"
>> +#include "hw/nmi.h"
>> +
>> +typedef struct {
>> +    /*< private >*/
>> +    MachineClass parent;
>> +
>> +    /*< public >*/
>> +
>> +    /* Enables contiguous-apic-ID mode */
>> +    bool compat_apic_id_mode;
>> +} X86MachineClass;
>> +
>> +typedef struct {
>> +    /*< private >*/
>> +    MachineState parent;
>> +
>> +    /*< public >*/
>> +
>> +    /* Pointers to devices and objects: */
>> +    ISADevice *rtc;
>> +    FWCfgState *fw_cfg;
>> +    qemu_irq *gsi;
>> +    GMappedFile *initrd_mapped_file;
>> +
>> +    /* Configuration options: */
>> +    uint64_t max_ram_below_4g;
>> +
>> +    /* RAM information (sizes, addresses, configuration): */
>> +    ram_addr_t below_4g_mem_size, above_4g_mem_size;
>> +
>> +    /* CPU and apic information: */
>> +    bool apic_xrupt_override;
>> +    unsigned apic_id_limit;
>> +    uint16_t boot_cpus;
>> +    unsigned smp_dies;
>> +
>> +    /* Address space used by IOAPIC device. All IOAPIC interrupts
>> +     * will be translated to MSI messages in the address space. */
>> +    AddressSpace *ioapic_as;
>> +} X86MachineState;
>> +
>> +#define X86_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
>> +
>> +#define TYPE_X86_MACHINE   MACHINE_TYPE_NAME("x86")
>> +#define X86_MACHINE(obj) \
>> +    OBJECT_CHECK(X86MachineState, (obj), TYPE_X86_MACHINE)
>> +#define X86_MACHINE_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(X86MachineClass, obj, TYPE_X86_MACHINE)
>> +#define X86_MACHINE_CLASS(class) \
>> +    OBJECT_CLASS_CHECK(X86MachineClass, class, TYPE_X86_MACHINE)
>>     uint32_t x86_cpu_apic_id_from_index(PCMachineState *pcms,
>>                                       unsigned int cpu_index);
>> @@ -30,6 +84,6 @@ const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms);
>>     void x86_system_rom_init(MemoryRegion *rom_memory, bool
>> isapc_ram_fw);
>>   -void x86_load_linux(PCMachineState *x86ms, FWCfgState *fw_cfg);
>> +void x86_load_linux(PCMachineState *pcms, FWCfgState *fw_cfg);
>>     #endif
>>
>
> You forgot to update Xen:
>
> hw/i386/xen/xen-hvm.c: In function ‘xen_ram_init’:
> hw/i386/xen/xen-hvm.c:203:53: error: ‘PC_MACHINE_MAX_RAM_BELOW_4G’
> undeclared (first use in this function); did you mean
> ‘X86_MACHINE_MAX_RAM_BELOW_4G’?
>
> PC_MACHINE_MAX_RAM_BELOW_4G,
>
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> X86_MACHINE_MAX_RAM_BELOW_4G
> hw/i386/xen/xen-hvm.c:203:53: note: each undeclared identifier is
> reported only once for each function it appears in
> hw/i386/xen/xen-hvm.c:217:13: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘above_4g_mem_size’
>          pcms->above_4g_mem_size = ram_size - user_lowmem;
>              ^~
> hw/i386/xen/xen-hvm.c:218:13: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘below_4g_mem_size’
>          pcms->below_4g_mem_size = user_lowmem;
>              ^~
> hw/i386/xen/xen-hvm.c:220:13: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘above_4g_mem_size’
>          pcms->above_4g_mem_size = 0;
>              ^~
> hw/i386/xen/xen-hvm.c:221:13: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘below_4g_mem_size’
>          pcms->below_4g_mem_size = ram_size;
>              ^~
> hw/i386/xen/xen-hvm.c:223:14: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘above_4g_mem_size’
>      if (!pcms->above_4g_mem_size) {
>               ^~
> hw/i386/xen/xen-hvm.c:230:40: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘above_4g_mem_size’
>          block_len = (1ULL << 32) + pcms->above_4g_mem_size;
>                                         ^~
> hw/i386/xen/xen-hvm.c:247:34: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘below_4g_mem_size’
>                               pcms->below_4g_mem_size - 0xc0000);
>                                   ^~
> hw/i386/xen/xen-hvm.c:249:13: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘above_4g_mem_size’
>      if (pcms->above_4g_mem_size > 0) {
>              ^~
> hw/i386/xen/xen-hvm.c:252:38: error: ‘PCMachineState’ {aka ‘struct
> PCMachineState’} has no member named ‘above_4g_mem_size’
>                                   pcms->above_4g_mem_size);
>                                       ^~
> make[1]: *** [rules.mak:69: hw/i386/xen/xen-hvm.o] Error 1

Ouch, turns out I forgot to install xen-devel when I renewed the VM that
I'm using for this project.

I'll fix it right now.

Thanks,
Sergio.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]