[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to scl
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h |
Date: |
Mon, 11 Sep 2017 15:45:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 11.09.2017 12:23, Paolo Bonzini wrote:
> On 10/09/2017 00:07, Eduardo Habkost wrote:
>> On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:
>>> On 08.09.2017 06:21, Thomas Huth wrote:
>>>> On 07.09.2017 22:13, David Hildenbrand wrote:
>>>>> Implemented in sclp.c, so let's move it to the right include file.
>>>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the
>>>>> two sclp consoles complaining.
>>>>>
>>>>> Signed-off-by: David Hildenbrand <address@hidden>
>>>>> ---
>>>>> include/hw/s390x/sclp.h | 2 ++
>>>>> target/s390x/cpu.h | 1 -
>>>>> target/s390x/misc_helper.c | 1 +
>>>>> 3 files changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>>>>> index a72d096081..4b86a8a293 100644
>>>>> --- a/include/hw/s390x/sclp.h
>>>>> +++ b/include/hw/s390x/sclp.h
>>>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev
>>>>> *init_sclp_memory_hotplug_dev(void);
>>>>> sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
>>>>> void sclp_service_interrupt(uint32_t sccb);
>>>>> void raise_irq_cpu_hotplug(void);
>>>>> +typedef struct CPUS390XState CPUS390XState;
>>>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
>>>>
>>>> That's dangerous and likely does not work with certain versions of GCC.
>>>> You can't do a "forward declaration" with typedef in C, I'm afraid. See
>>>> for example:
>>>>
>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html
>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html
>>>> https://stackoverflow.com/questions/8367646/redefinition-of-typedef
>>>>
>>>> All this typedef'ing in QEMU is pretty bad ... we run into this problem
>>>> again and again. include/qemu/typedefs.h is just a work-around for this.
>>>> I know people like typedefs for some reasons (I used to do that, too,
>>>> before I realized the trouble with them), but IMHO we should rather
>>>> adopt the typedef-related rules from the kernel coding conventions instead:
>>>>
>>>> https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs
>>>>
>>>> Thomas
>>>>
>>>
>>> Yes, this is really nasty. And I wasn't aware of the involved issues.
>>>
>>> This seems to be the only feasible solution (including cpu.h sounds
>>> wrong and will require a bunch of other includes):
>>>
>>>
>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>>> index a72d096081..ce80915a02 100644
>>> --- a/include/hw/s390x/sclp.h
>>> +++ b/include/hw/s390x/sclp.h
>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev
>>> *init_sclp_memory_hotplug_dev(void);
>>> sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
>>> void sclp_service_interrupt(uint32_t sccb);
>>> void raise_irq_cpu_hotplug(void);
>>> +struct CPUS390XState;
>>> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,
>>> uint32_t code);
>>>
>>> #endif
>>
>> Why not use typedefs.h?
>
> See Markus's reply. But, maybe it's even better to use S390CPU* and
> include target/s390x/cpu-qom.h, which by design provides as little
> definitions as needed.
I'll go with that approach. I'll drop the dependency from cpu-qom.h to
cpu_models.h (by moving typedefs to cpu-qom.h). This makes it compile at
hopefully should then be good enough for now.
(this approach implies dropping patch "[PATCH v3 05/21] target/s390x:
move typedef of S390CPU to its definition").
Thanks!
>
>> Signed-off-by: Eduardo Habkost <address@hidden>
--
Thanks,
David
- Re: [Qemu-devel] [PATCH v3 07/21] s390x: move subsystem_reset() to s390-virtio-ccw.h, (continued)
- [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, David Hildenbrand, 2017/09/07
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, Thomas Huth, 2017/09/08
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, David Hildenbrand, 2017/09/08
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, Eduardo Habkost, 2017/09/09
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, Thomas Huth, 2017/09/10
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, Paolo Bonzini, 2017/09/11
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h,
David Hildenbrand <=
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, Eduardo Habkost, 2017/09/11
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, David Hildenbrand, 2017/09/11
- Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h, Eduardo Habkost, 2017/09/11
[Qemu-devel] [PATCH v3 09/21] target/s390x: use trigger_pgm_exception() in s390_cpu_handle_mmu_fault(), David Hildenbrand, 2017/09/07
[Qemu-devel] [PATCH v3 10/21] target/s390x: use program_interrupt() in per_check_exception(), David Hildenbrand, 2017/09/07
[Qemu-devel] [PATCH v3 11/21] s390x: allow only 1 CPU with TCG, David Hildenbrand, 2017/09/07
[Qemu-devel] [PATCH v3 12/21] target/s390x: set cpu->id for linux user when realizing, David Hildenbrand, 2017/09/07
[Qemu-devel] [PATCH v3 13/21] target/s390x: use "core-id" for cpu number/address/id handling, David Hildenbrand, 2017/09/07
[Qemu-devel] [PATCH v3 14/21] target/s390x: rename next_cpu_id to next_core_id, David Hildenbrand, 2017/09/07
[Qemu-devel] [PATCH v3 15/21] s390x: print CPU definitions in sorted order, David Hildenbrand, 2017/09/07