diff mbox series

[v3,1/9] target/i386: Restrict X86CPUFeatureWord to X86 targets

Message ID 20200525150640.30879-2-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series user-mode: Prune build dependencies (part 2) | expand

Commit Message

Philippe Mathieu-Daudé May 25, 2020, 3:06 p.m. UTC
Move out x86-specific structures from generic machine code.

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
 qapi/machine.json          | 42 -----------------------------------
 target/i386/cpu.c          |  2 +-
 target/i386/machine-stub.c | 22 +++++++++++++++++++
 target/i386/Makefile.objs  |  3 ++-
 5 files changed, 70 insertions(+), 44 deletions(-)
 create mode 100644 target/i386/machine-stub.c

Comments

Markus Armbruster May 26, 2020, 6:45 a.m. UTC | #1
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Move out x86-specific structures from generic machine code.
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
>  qapi/machine.json          | 42 -----------------------------------
>  target/i386/cpu.c          |  2 +-
>  target/i386/machine-stub.c | 22 +++++++++++++++++++
>  target/i386/Makefile.objs  |  3 ++-
>  5 files changed, 70 insertions(+), 44 deletions(-)
>  create mode 100644 target/i386/machine-stub.c
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index f2c82949d8..fb7a4b7850 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -3,6 +3,51 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>  # See the COPYING file in the top-level directory.
>  
> +##
> +# @X86CPURegister32:
> +#
> +# A X86 32-bit register
> +#
> +# Since: 1.5
> +##
> +{ 'enum': 'X86CPURegister32',
> +  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
> +  'if': 'defined(TARGET_I386)' }
> +
> +##
> +# @X86CPUFeatureWordInfo:
> +#
> +# Information about a X86 CPU feature word
> +#
> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
> +#
> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
> +#                   feature word
> +#
> +# @cpuid-register: Output register containing the feature bits
> +#
> +# @features: value of output register, containing the feature bits
> +#
> +# Since: 1.5
> +##
> +{ 'struct': 'X86CPUFeatureWordInfo',
> +  'data': { 'cpuid-input-eax': 'int',
> +            '*cpuid-input-ecx': 'int',
> +            'cpuid-register': 'X86CPURegister32',
> +            'features': 'int' },
> +  'if': 'defined(TARGET_I386)' }
> +
> +##
> +# @DummyForceArrays:
> +#
> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'DummyForceArrays',
> +  'data': { 'unused': ['X86CPUFeatureWordInfo'] },
> +  'if': 'defined(TARGET_I386)' }
> +
>  ##
>  # @CpuModelInfo:
>  #
> diff --git a/qapi/machine.json b/qapi/machine.json
> index ff7b5032e3..1fe31d374c 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -511,48 +511,6 @@
>     'dst': 'uint16',
>     'val': 'uint8' }}
>  
> -##
> -# @X86CPURegister32:
> -#
> -# A X86 32-bit register
> -#
> -# Since: 1.5
> -##
> -{ 'enum': 'X86CPURegister32',
> -  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
> -
> -##
> -# @X86CPUFeatureWordInfo:
> -#
> -# Information about a X86 CPU feature word
> -#
> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
> -#
> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
> -#                   feature word
> -#
> -# @cpuid-register: Output register containing the feature bits
> -#
> -# @features: value of output register, containing the feature bits
> -#
> -# Since: 1.5
> -##
> -{ 'struct': 'X86CPUFeatureWordInfo',
> -  'data': { 'cpuid-input-eax': 'int',
> -            '*cpuid-input-ecx': 'int',
> -            'cpuid-register': 'X86CPURegister32',
> -            'features': 'int' } }
> -
> -##
> -# @DummyForceArrays:
> -#
> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
> -#
> -# Since: 2.5
> -##
> -{ 'struct': 'DummyForceArrays',
> -  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
> -
>  ##
>  # @NumaCpuOptions:
>  #
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 7a4a8e3847..832498c723 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -37,7 +37,7 @@
>  #include "qemu/option.h"
>  #include "qemu/config-file.h"
>  #include "qapi/error.h"
> -#include "qapi/qapi-visit-machine.h"
> +#include "qapi/qapi-visit-machine-target.h"
>  #include "qapi/qapi-visit-run-state.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
> diff --git a/target/i386/machine-stub.c b/target/i386/machine-stub.c
> new file mode 100644
> index 0000000000..cb301af057
> --- /dev/null
> +++ b/target/i386/machine-stub.c
> @@ -0,0 +1,22 @@
> +/*
> + * QAPI x86 CPU features stub
> + *
> + * Copyright (c) 2020 Red Hat, Inc.
> + *
> + * Author:
> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-visit-machine-target.h"
> +
> +void visit_type_X86CPUFeatureWordInfoList(Visitor *v, const char *name,
> +                                      X86CPUFeatureWordInfoList **obj,
> +                                      Error **errp)

Unusual indentation.

> +{
> +}

Two kinds of stubs: stubs that can get called, and stubs that exist only
to satisfy the linker.  Which kind is this one?

> diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs
> index 48e0c28434..1cdfc9f50c 100644
> --- a/target/i386/Makefile.objs
> +++ b/target/i386/Makefile.objs
> @@ -17,6 +17,7 @@ obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-posix.o
>  endif
>  obj-$(CONFIG_HVF) += hvf/
>  obj-$(CONFIG_WHPX) += whpx-all.o
> -endif
> +endif # CONFIG_SOFTMMU
>  obj-$(CONFIG_SEV) += sev.o
>  obj-$(call lnot,$(CONFIG_SEV)) += sev-stub.o
> +obj-$(call lnot,$(CONFIG_SOFTMMU)) += machine-stub.o

Suggest

   ifeq ($(CONFIG_SOFTMMU),y)
   ...
   else
   obj-y += machine-stub.o
   endif

Matter of taste.
Philippe Mathieu-Daudé May 26, 2020, 7:23 a.m. UTC | #2
On 5/26/20 8:45 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> Move out x86-specific structures from generic machine code.
>>
>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
>>  qapi/machine.json          | 42 -----------------------------------
>>  target/i386/cpu.c          |  2 +-
>>  target/i386/machine-stub.c | 22 +++++++++++++++++++
>>  target/i386/Makefile.objs  |  3 ++-
>>  5 files changed, 70 insertions(+), 44 deletions(-)
>>  create mode 100644 target/i386/machine-stub.c
>>
>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>> index f2c82949d8..fb7a4b7850 100644
>> --- a/qapi/machine-target.json
>> +++ b/qapi/machine-target.json
>> @@ -3,6 +3,51 @@
>>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>  # See the COPYING file in the top-level directory.
>>  
>> +##
>> +# @X86CPURegister32:
>> +#
>> +# A X86 32-bit register
>> +#
>> +# Since: 1.5
>> +##
>> +{ 'enum': 'X86CPURegister32',
>> +  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
>> +  'if': 'defined(TARGET_I386)' }
>> +
>> +##
>> +# @X86CPUFeatureWordInfo:
>> +#
>> +# Information about a X86 CPU feature word
>> +#
>> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>> +#
>> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>> +#                   feature word
>> +#
>> +# @cpuid-register: Output register containing the feature bits
>> +#
>> +# @features: value of output register, containing the feature bits
>> +#
>> +# Since: 1.5
>> +##
>> +{ 'struct': 'X86CPUFeatureWordInfo',
>> +  'data': { 'cpuid-input-eax': 'int',
>> +            '*cpuid-input-ecx': 'int',
>> +            'cpuid-register': 'X86CPURegister32',
>> +            'features': 'int' },
>> +  'if': 'defined(TARGET_I386)' }
>> +
>> +##
>> +# @DummyForceArrays:
>> +#
>> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'DummyForceArrays',
>> +  'data': { 'unused': ['X86CPUFeatureWordInfo'] },
>> +  'if': 'defined(TARGET_I386)' }
>> +
>>  ##
>>  # @CpuModelInfo:
>>  #
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index ff7b5032e3..1fe31d374c 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -511,48 +511,6 @@
>>     'dst': 'uint16',
>>     'val': 'uint8' }}
>>  
>> -##
>> -# @X86CPURegister32:
>> -#
>> -# A X86 32-bit register
>> -#
>> -# Since: 1.5
>> -##
>> -{ 'enum': 'X86CPURegister32',
>> -  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
>> -
>> -##
>> -# @X86CPUFeatureWordInfo:
>> -#
>> -# Information about a X86 CPU feature word
>> -#
>> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>> -#
>> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>> -#                   feature word
>> -#
>> -# @cpuid-register: Output register containing the feature bits
>> -#
>> -# @features: value of output register, containing the feature bits
>> -#
>> -# Since: 1.5
>> -##
>> -{ 'struct': 'X86CPUFeatureWordInfo',
>> -  'data': { 'cpuid-input-eax': 'int',
>> -            '*cpuid-input-ecx': 'int',
>> -            'cpuid-register': 'X86CPURegister32',
>> -            'features': 'int' } }
>> -
>> -##
>> -# @DummyForceArrays:
>> -#
>> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>> -#
>> -# Since: 2.5
>> -##
>> -{ 'struct': 'DummyForceArrays',
>> -  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
>> -
>>  ##
>>  # @NumaCpuOptions:
>>  #
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 7a4a8e3847..832498c723 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -37,7 +37,7 @@
>>  #include "qemu/option.h"
>>  #include "qemu/config-file.h"
>>  #include "qapi/error.h"
>> -#include "qapi/qapi-visit-machine.h"
>> +#include "qapi/qapi-visit-machine-target.h"
>>  #include "qapi/qapi-visit-run-state.h"
>>  #include "qapi/qmp/qdict.h"
>>  #include "qapi/qmp/qerror.h"
>> diff --git a/target/i386/machine-stub.c b/target/i386/machine-stub.c
>> new file mode 100644
>> index 0000000000..cb301af057
>> --- /dev/null
>> +++ b/target/i386/machine-stub.c
>> @@ -0,0 +1,22 @@
>> +/*
>> + * QAPI x86 CPU features stub
>> + *
>> + * Copyright (c) 2020 Red Hat, Inc.
>> + *
>> + * Author:
>> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "qapi/qapi-visit-machine-target.h"
>> +
>> +void visit_type_X86CPUFeatureWordInfoList(Visitor *v, const char *name,
>> +                                      X86CPUFeatureWordInfoList **obj,
>> +                                      Error **errp)
> 
> Unusual indentation.
> 
>> +{
>> +}
> 
> Two kinds of stubs: stubs that can get called, and stubs that exist only
> to satisfy the linker.  Which kind is this one?

I suppose "stubs that exist only to satisfy the linker" must abort if
ever called...

> 
>> diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs
>> index 48e0c28434..1cdfc9f50c 100644
>> --- a/target/i386/Makefile.objs
>> +++ b/target/i386/Makefile.objs
>> @@ -17,6 +17,7 @@ obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-posix.o
>>  endif
>>  obj-$(CONFIG_HVF) += hvf/
>>  obj-$(CONFIG_WHPX) += whpx-all.o
>> -endif
>> +endif # CONFIG_SOFTMMU
>>  obj-$(CONFIG_SEV) += sev.o
>>  obj-$(call lnot,$(CONFIG_SEV)) += sev-stub.o
>> +obj-$(call lnot,$(CONFIG_SOFTMMU)) += machine-stub.o
> 
> Suggest
> 
>    ifeq ($(CONFIG_SOFTMMU),y)
>    ...
>    else
>    obj-y += machine-stub.o
>    endif
> 
> Matter of taste.
> 

OK
Philippe Mathieu-Daudé May 26, 2020, 7:36 a.m. UTC | #3
On 5/26/20 9:23 AM, Philippe Mathieu-Daudé wrote:
> On 5/26/20 8:45 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> Move out x86-specific structures from generic machine code.
>>>
>>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
>>>  qapi/machine.json          | 42 -----------------------------------
>>>  target/i386/cpu.c          |  2 +-
>>>  target/i386/machine-stub.c | 22 +++++++++++++++++++
>>>  target/i386/Makefile.objs  |  3 ++-
>>>  5 files changed, 70 insertions(+), 44 deletions(-)
>>>  create mode 100644 target/i386/machine-stub.c
>>>
>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>> index f2c82949d8..fb7a4b7850 100644
>>> --- a/qapi/machine-target.json
>>> +++ b/qapi/machine-target.json
>>> @@ -3,6 +3,51 @@
>>>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>  # See the COPYING file in the top-level directory.
>>>  
>>> +##
>>> +# @X86CPURegister32:
>>> +#
>>> +# A X86 32-bit register
>>> +#
>>> +# Since: 1.5
>>> +##
>>> +{ 'enum': 'X86CPURegister32',
>>> +  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
>>> +  'if': 'defined(TARGET_I386)' }
>>> +
>>> +##
>>> +# @X86CPUFeatureWordInfo:
>>> +#
>>> +# Information about a X86 CPU feature word
>>> +#
>>> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>>> +#
>>> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>>> +#                   feature word
>>> +#
>>> +# @cpuid-register: Output register containing the feature bits
>>> +#
>>> +# @features: value of output register, containing the feature bits
>>> +#
>>> +# Since: 1.5
>>> +##
>>> +{ 'struct': 'X86CPUFeatureWordInfo',
>>> +  'data': { 'cpuid-input-eax': 'int',
>>> +            '*cpuid-input-ecx': 'int',
>>> +            'cpuid-register': 'X86CPURegister32',
>>> +            'features': 'int' },
>>> +  'if': 'defined(TARGET_I386)' }
>>> +
>>> +##
>>> +# @DummyForceArrays:
>>> +#
>>> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>>> +#
>>> +# Since: 2.5
>>> +##
>>> +{ 'struct': 'DummyForceArrays',
>>> +  'data': { 'unused': ['X86CPUFeatureWordInfo'] },
>>> +  'if': 'defined(TARGET_I386)' }
>>> +
>>>  ##
>>>  # @CpuModelInfo:
>>>  #
>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index ff7b5032e3..1fe31d374c 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json
>>> @@ -511,48 +511,6 @@
>>>     'dst': 'uint16',
>>>     'val': 'uint8' }}
>>>  
>>> -##
>>> -# @X86CPURegister32:
>>> -#
>>> -# A X86 32-bit register
>>> -#
>>> -# Since: 1.5
>>> -##
>>> -{ 'enum': 'X86CPURegister32',
>>> -  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
>>> -
>>> -##
>>> -# @X86CPUFeatureWordInfo:
>>> -#
>>> -# Information about a X86 CPU feature word
>>> -#
>>> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>>> -#
>>> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>>> -#                   feature word
>>> -#
>>> -# @cpuid-register: Output register containing the feature bits
>>> -#
>>> -# @features: value of output register, containing the feature bits
>>> -#
>>> -# Since: 1.5
>>> -##
>>> -{ 'struct': 'X86CPUFeatureWordInfo',
>>> -  'data': { 'cpuid-input-eax': 'int',
>>> -            '*cpuid-input-ecx': 'int',
>>> -            'cpuid-register': 'X86CPURegister32',
>>> -            'features': 'int' } }
>>> -
>>> -##
>>> -# @DummyForceArrays:
>>> -#
>>> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>>> -#
>>> -# Since: 2.5
>>> -##
>>> -{ 'struct': 'DummyForceArrays',
>>> -  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
>>> -
>>>  ##
>>>  # @NumaCpuOptions:
>>>  #
>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>> index 7a4a8e3847..832498c723 100644
>>> --- a/target/i386/cpu.c
>>> +++ b/target/i386/cpu.c
>>> @@ -37,7 +37,7 @@
>>>  #include "qemu/option.h"
>>>  #include "qemu/config-file.h"
>>>  #include "qapi/error.h"
>>> -#include "qapi/qapi-visit-machine.h"
>>> +#include "qapi/qapi-visit-machine-target.h"
>>>  #include "qapi/qapi-visit-run-state.h"
>>>  #include "qapi/qmp/qdict.h"
>>>  #include "qapi/qmp/qerror.h"
>>> diff --git a/target/i386/machine-stub.c b/target/i386/machine-stub.c
>>> new file mode 100644
>>> index 0000000000..cb301af057
>>> --- /dev/null
>>> +++ b/target/i386/machine-stub.c
>>> @@ -0,0 +1,22 @@
>>> +/*
>>> + * QAPI x86 CPU features stub
>>> + *
>>> + * Copyright (c) 2020 Red Hat, Inc.
>>> + *
>>> + * Author:
>>> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qapi/error.h"
>>> +#include "qapi/qapi-visit-machine-target.h"
>>> +
>>> +void visit_type_X86CPUFeatureWordInfoList(Visitor *v, const char *name,
>>> +                                      X86CPUFeatureWordInfoList **obj,
>>> +                                      Error **errp)
>>
>> Unusual indentation.
>>
>>> +{
>>> +}
>>
>> Two kinds of stubs: stubs that can get called, and stubs that exist only
>> to satisfy the linker.  Which kind is this one?
> 
> I suppose "stubs that exist only to satisfy the linker" must abort if
> ever called...

Link problem when building linux-user:

  LINK    i386-linux-user/qemu-i386
/usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_get_feature_words':
target/i386/cpu.c:4627: undefined reference to
`visit_type_X86CPUFeatureWordInfoList'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:208: qemu-i386] Error 1
make: *** [Makefile:527: i386-linux-user/all] Error 2

Various properties from x86_cpu_initfn() should be removed from
user-mode (tsc/stepping/kvm/...) but I'll let that to some x86 specialist.

> 
>>
>>> diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs
>>> index 48e0c28434..1cdfc9f50c 100644
>>> --- a/target/i386/Makefile.objs
>>> +++ b/target/i386/Makefile.objs
>>> @@ -17,6 +17,7 @@ obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-posix.o
>>>  endif
>>>  obj-$(CONFIG_HVF) += hvf/
>>>  obj-$(CONFIG_WHPX) += whpx-all.o
>>> -endif
>>> +endif # CONFIG_SOFTMMU
>>>  obj-$(CONFIG_SEV) += sev.o
>>>  obj-$(call lnot,$(CONFIG_SEV)) += sev-stub.o
>>> +obj-$(call lnot,$(CONFIG_SOFTMMU)) += machine-stub.o
>>
>> Suggest
>>
>>    ifeq ($(CONFIG_SOFTMMU),y)
>>    ...
>>    else
>>    obj-y += machine-stub.o
>>    endif
>>
>> Matter of taste.
>>
> 
> OK
>
Markus Armbruster May 26, 2020, 9:02 a.m. UTC | #4
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/26/20 9:23 AM, Philippe Mathieu-Daudé wrote:
>> On 5/26/20 8:45 AM, Markus Armbruster wrote:
>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>
>>>> Move out x86-specific structures from generic machine code.
>>>>
>>>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
>>>>  qapi/machine.json          | 42 -----------------------------------
>>>>  target/i386/cpu.c          |  2 +-
>>>>  target/i386/machine-stub.c | 22 +++++++++++++++++++
>>>>  target/i386/Makefile.objs  |  3 ++-
>>>>  5 files changed, 70 insertions(+), 44 deletions(-)
>>>>  create mode 100644 target/i386/machine-stub.c
>>>>
>>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>>> index f2c82949d8..fb7a4b7850 100644
>>>> --- a/qapi/machine-target.json
>>>> +++ b/qapi/machine-target.json
>>>> @@ -3,6 +3,51 @@
>>>>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>>  # See the COPYING file in the top-level directory.
>>>>  
>>>> +##
>>>> +# @X86CPURegister32:
>>>> +#
>>>> +# A X86 32-bit register
>>>> +#
>>>> +# Since: 1.5
>>>> +##
>>>> +{ 'enum': 'X86CPURegister32',
>>>> +  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
>>>> +  'if': 'defined(TARGET_I386)' }
>>>> +
>>>> +##
>>>> +# @X86CPUFeatureWordInfo:
>>>> +#
>>>> +# Information about a X86 CPU feature word
>>>> +#
>>>> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>>>> +#
>>>> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>>>> +#                   feature word
>>>> +#
>>>> +# @cpuid-register: Output register containing the feature bits
>>>> +#
>>>> +# @features: value of output register, containing the feature bits
>>>> +#
>>>> +# Since: 1.5
>>>> +##
>>>> +{ 'struct': 'X86CPUFeatureWordInfo',
>>>> +  'data': { 'cpuid-input-eax': 'int',
>>>> +            '*cpuid-input-ecx': 'int',
>>>> +            'cpuid-register': 'X86CPURegister32',
>>>> +            'features': 'int' },
>>>> +  'if': 'defined(TARGET_I386)' }
>>>> +
>>>> +##
>>>> +# @DummyForceArrays:
>>>> +#
>>>> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>>>> +#
>>>> +# Since: 2.5
>>>> +##
>>>> +{ 'struct': 'DummyForceArrays',
>>>> +  'data': { 'unused': ['X86CPUFeatureWordInfo'] },
>>>> +  'if': 'defined(TARGET_I386)' }
>>>> +
>>>>  ##
>>>>  # @CpuModelInfo:
>>>>  #
>>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>>> index ff7b5032e3..1fe31d374c 100644
>>>> --- a/qapi/machine.json
>>>> +++ b/qapi/machine.json
>>>> @@ -511,48 +511,6 @@
>>>>     'dst': 'uint16',
>>>>     'val': 'uint8' }}
>>>>  
>>>> -##
>>>> -# @X86CPURegister32:
>>>> -#
>>>> -# A X86 32-bit register
>>>> -#
>>>> -# Since: 1.5
>>>> -##
>>>> -{ 'enum': 'X86CPURegister32',
>>>> -  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
>>>> -
>>>> -##
>>>> -# @X86CPUFeatureWordInfo:
>>>> -#
>>>> -# Information about a X86 CPU feature word
>>>> -#
>>>> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>>>> -#
>>>> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>>>> -#                   feature word
>>>> -#
>>>> -# @cpuid-register: Output register containing the feature bits
>>>> -#
>>>> -# @features: value of output register, containing the feature bits
>>>> -#
>>>> -# Since: 1.5
>>>> -##
>>>> -{ 'struct': 'X86CPUFeatureWordInfo',
>>>> -  'data': { 'cpuid-input-eax': 'int',
>>>> -            '*cpuid-input-ecx': 'int',
>>>> -            'cpuid-register': 'X86CPURegister32',
>>>> -            'features': 'int' } }
>>>> -
>>>> -##
>>>> -# @DummyForceArrays:
>>>> -#
>>>> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>>>> -#
>>>> -# Since: 2.5
>>>> -##
>>>> -{ 'struct': 'DummyForceArrays',
>>>> -  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
>>>> -
>>>>  ##
>>>>  # @NumaCpuOptions:
>>>>  #
>>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>>> index 7a4a8e3847..832498c723 100644
>>>> --- a/target/i386/cpu.c
>>>> +++ b/target/i386/cpu.c
>>>> @@ -37,7 +37,7 @@
>>>>  #include "qemu/option.h"
>>>>  #include "qemu/config-file.h"
>>>>  #include "qapi/error.h"
>>>> -#include "qapi/qapi-visit-machine.h"
>>>> +#include "qapi/qapi-visit-machine-target.h"
>>>>  #include "qapi/qapi-visit-run-state.h"
>>>>  #include "qapi/qmp/qdict.h"
>>>>  #include "qapi/qmp/qerror.h"
>>>> diff --git a/target/i386/machine-stub.c b/target/i386/machine-stub.c
>>>> new file mode 100644
>>>> index 0000000000..cb301af057
>>>> --- /dev/null
>>>> +++ b/target/i386/machine-stub.c
>>>> @@ -0,0 +1,22 @@
>>>> +/*
>>>> + * QAPI x86 CPU features stub
>>>> + *
>>>> + * Copyright (c) 2020 Red Hat, Inc.
>>>> + *
>>>> + * Author:
>>>> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>> + * See the COPYING file in the top-level directory.
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "qapi/error.h"
>>>> +#include "qapi/qapi-visit-machine-target.h"
>>>> +
>>>> +void visit_type_X86CPUFeatureWordInfoList(Visitor *v, const char *name,
>>>> +                                      X86CPUFeatureWordInfoList **obj,
>>>> +                                      Error **errp)
>>>
>>> Unusual indentation.
>>>
>>>> +{
>>>> +}
>>>
>>> Two kinds of stubs: stubs that can get called, and stubs that exist only
>>> to satisfy the linker.  Which kind is this one?
>> 
>> I suppose "stubs that exist only to satisfy the linker" must abort if
>> ever called...

Yes, and having such stubs can be okay.

> Link problem when building linux-user:
>
>   LINK    i386-linux-user/qemu-i386
> /usr/bin/ld: target/i386/cpu.o: in function `x86_cpu_get_feature_words':
> target/i386/cpu.c:4627: undefined reference to
> `visit_type_X86CPUFeatureWordInfoList'
> collect2: error: ld returned 1 exit status
> make[1]: *** [Makefile:208: qemu-i386] Error 1
> make: *** [Makefile:527: i386-linux-user/all] Error 2
>
> Various properties from x86_cpu_initfn() should be removed from
> user-mode (tsc/stepping/kvm/...) but I'll let that to some x86 specialist.

No objection.  Just tell me whether the stub is meant to be called,
because if it is, we need to consider what it should do when called.

[...]
diff mbox series

Patch

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index f2c82949d8..fb7a4b7850 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -3,6 +3,51 @@ 
 # This work is licensed under the terms of the GNU GPL, version 2 or later.
 # See the COPYING file in the top-level directory.
 
+##
+# @X86CPURegister32:
+#
+# A X86 32-bit register
+#
+# Since: 1.5
+##
+{ 'enum': 'X86CPURegister32',
+  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
+  'if': 'defined(TARGET_I386)' }
+
+##
+# @X86CPUFeatureWordInfo:
+#
+# Information about a X86 CPU feature word
+#
+# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
+#
+# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
+#                   feature word
+#
+# @cpuid-register: Output register containing the feature bits
+#
+# @features: value of output register, containing the feature bits
+#
+# Since: 1.5
+##
+{ 'struct': 'X86CPUFeatureWordInfo',
+  'data': { 'cpuid-input-eax': 'int',
+            '*cpuid-input-ecx': 'int',
+            'cpuid-register': 'X86CPURegister32',
+            'features': 'int' },
+  'if': 'defined(TARGET_I386)' }
+
+##
+# @DummyForceArrays:
+#
+# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
+#
+# Since: 2.5
+##
+{ 'struct': 'DummyForceArrays',
+  'data': { 'unused': ['X86CPUFeatureWordInfo'] },
+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @CpuModelInfo:
 #
diff --git a/qapi/machine.json b/qapi/machine.json
index ff7b5032e3..1fe31d374c 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -511,48 +511,6 @@ 
    'dst': 'uint16',
    'val': 'uint8' }}
 
-##
-# @X86CPURegister32:
-#
-# A X86 32-bit register
-#
-# Since: 1.5
-##
-{ 'enum': 'X86CPURegister32',
-  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
-
-##
-# @X86CPUFeatureWordInfo:
-#
-# Information about a X86 CPU feature word
-#
-# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
-#
-# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
-#                   feature word
-#
-# @cpuid-register: Output register containing the feature bits
-#
-# @features: value of output register, containing the feature bits
-#
-# Since: 1.5
-##
-{ 'struct': 'X86CPUFeatureWordInfo',
-  'data': { 'cpuid-input-eax': 'int',
-            '*cpuid-input-ecx': 'int',
-            'cpuid-register': 'X86CPURegister32',
-            'features': 'int' } }
-
-##
-# @DummyForceArrays:
-#
-# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
-#
-# Since: 2.5
-##
-{ 'struct': 'DummyForceArrays',
-  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
-
 ##
 # @NumaCpuOptions:
 #
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7a4a8e3847..832498c723 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -37,7 +37,7 @@ 
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "qapi/error.h"
-#include "qapi/qapi-visit-machine.h"
+#include "qapi/qapi-visit-machine-target.h"
 #include "qapi/qapi-visit-run-state.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
diff --git a/target/i386/machine-stub.c b/target/i386/machine-stub.c
new file mode 100644
index 0000000000..cb301af057
--- /dev/null
+++ b/target/i386/machine-stub.c
@@ -0,0 +1,22 @@ 
+/*
+ * QAPI x86 CPU features stub
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ *
+ * Author:
+ *   Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-visit-machine-target.h"
+
+void visit_type_X86CPUFeatureWordInfoList(Visitor *v, const char *name,
+                                      X86CPUFeatureWordInfoList **obj,
+                                      Error **errp)
+{
+}
diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs
index 48e0c28434..1cdfc9f50c 100644
--- a/target/i386/Makefile.objs
+++ b/target/i386/Makefile.objs
@@ -17,6 +17,7 @@  obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-posix.o
 endif
 obj-$(CONFIG_HVF) += hvf/
 obj-$(CONFIG_WHPX) += whpx-all.o
-endif
+endif # CONFIG_SOFTMMU
 obj-$(CONFIG_SEV) += sev.o
 obj-$(call lnot,$(CONFIG_SEV)) += sev-stub.o
+obj-$(call lnot,$(CONFIG_SOFTMMU)) += machine-stub.o