On 27.07.20 11:24, Cornelia Huck wrote:
> On Fri, 24 Jul 2020 16:37:44 +0200
> David Hildenbrand <da...@redhat.com> wrote:
>
>> Nowadays, we only have a single machine type in QEMU, everything is based
>> on virtio-ccw and the traditional virtio machine does no longer exist. No
>> need to dynamically register diag500 handlers. Move the two existing
>
> Hm, do we want to make certain subcodes available only if certain code
> has been configured? If yes, it might make sense to keep the mechanism.
I don't see a need to do that. And if it's a compile-time configuration
it can be handled within this file just fine (switch-case).
(the registration/call mechanism, including parameter passing and return
value handling is suboptimal for new subcodes.)
>
>> handlers into diag500.c.
>
> In any case, that file does not exist :)
Yeah, after reshuffling code 10 times ... thanks :)
>
>>
>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>> ---
>> hw/s390x/s390-virtio-ccw.c | 46 -----------------------------
>> hw/s390x/s390-virtio-hcall.c | 56 ++++++++++++++++++++++++------------
>> hw/s390x/s390-virtio-hcall.h | 2 --
>> 3 files changed, 38 insertions(+), 66 deletions(-)
>>
>
> (...)
>
>> diff --git a/hw/s390x/s390-virtio-hcall.c b/hw/s390x/s390-virtio-hcall.c
>> index ec7cf8beb3..5e14bd49b7 100644
>> --- a/hw/s390x/s390-virtio-hcall.c
>> +++ b/hw/s390x/s390-virtio-hcall.c
>> @@ -12,30 +12,50 @@
>> #include "qemu/osdep.h"
>> #include "cpu.h"
>> #include "hw/s390x/s390-virtio-hcall.h"
>> +#include "hw/s390x/ioinst.h"
>
> (Maybe you could remove the ioinst.h include from s390-virtio-ccw.c
> with this change?)
I think I tried and it wouldn't compile. But my memory might be wrong.
Will retry.
>
>> +#include "hw/s390x/css.h"
>> +#include "virtio-ccw.h"
>>
>> -#define MAX_DIAG_SUBCODES 255
>> -
>> -static s390_virtio_fn s390_diag500_table[MAX_DIAG_SUBCODES];
>> -
>> -void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn)
>> +static int handle_virtio_notify(uint64_t mem)
>> {
>> - assert(code < MAX_DIAG_SUBCODES);
>> - assert(!s390_diag500_table[code]);
>> -
>> - s390_diag500_table[code] = fn;
>> + if (mem < ram_size) {
>> + /* Tolerate early printk. */
>
> I'm wondering if we still need this. Probably doesn't hurt too much to
> keep it around, though.
Yeah, I just sticked to current code. Thanks!
--
Thanks,
David / dhildenb
