[v2,1/1] target/riscv/kvm: fix timebase-frequency when using KVM acceleration

Commit Message

Yong-Xuan Wang March 14, 2024, 6:15 a.m. UTC

The timebase-frequency of guest OS should be the same with host
machine. The timebase-frequency value in DTS should be got from
hypervisor when using KVM acceleration.

Reviewed-by: Andrew Jones <[e-mail]>
Signed-off-by: Yong-Xuan Wang <[e-mail]>

---
Changelog
v2:
- update the function definition
- restructure if-else statement
---
 hw/riscv/virt.c              | 2 ++
 target/riscv/kvm/kvm-cpu.c   | 9 +++++++++
 target/riscv/kvm/kvm_riscv.h | 1 +
 3 files changed, 12 insertions(+)

Comments

Philippe Mathieu-Daudé March 14, 2024, 6:55 a.m. UTC | #1

On 14/3/24 07:15, Yong-Xuan Wang wrote:
> The timebase-frequency of guest OS should be the same with host
> machine. The timebase-frequency value in DTS should be got from
> hypervisor when using KVM acceleration.
> 
> Reviewed-by: Andrew Jones <[e-mail]>
> Signed-off-by: Yong-Xuan Wang <[e-mail]>
> 
> ---
> Changelog
> v2:
> - update the function definition
> - restructure if-else statement

Thanks!

Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>

> ---
>   hw/riscv/virt.c              | 2 ++
>   target/riscv/kvm/kvm-cpu.c   | 9 +++++++++
>   target/riscv/kvm/kvm_riscv.h | 1 +
>   3 files changed, 12 insertions(+)

Alistair Francis March 22, 2024, 4:46 a.m. UTC | #2

On Thu, Mar 14, 2024 at 4:17 PM Yong-Xuan Wang <[e-mail]> wrote:
>
> The timebase-frequency of guest OS should be the same with host
> machine. The timebase-frequency value in DTS should be got from
> hypervisor when using KVM acceleration.
>
> Reviewed-by: Andrew Jones <[e-mail]>
> Signed-off-by: Yong-Xuan Wang <[e-mail]>

Reviewed-by: Alistair Francis <[e-mail]>

Alistair

>
> ---
> Changelog
> v2:
> - update the function definition
> - restructure if-else statement
> ---
>  hw/riscv/virt.c              | 2 ++
>  target/riscv/kvm/kvm-cpu.c   | 9 +++++++++
>  target/riscv/kvm/kvm_riscv.h | 1 +
>  3 files changed, 12 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a094af97c32a..533b17799581 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -711,6 +711,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>
>      qemu_fdt_add_subnode(ms->fdt, "/cpus");
>      qemu_fdt_setprop_cell(ms->fdt, "/cpus", "timebase-frequency",
> +                          kvm_enabled() ?
> +                          kvm_riscv_get_timebase_frequency(first_cpu) :
>                            RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ);
>      qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#size-cells", 0x0);
>      qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#address-cells", 0x1);
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c7afdb1e81b7..bbb115eaa867 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -739,6 +739,15 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
>      env->kvm_timer_dirty = false;
>  }
>
> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs)
> +{
> +    uint64_t reg;
> +
> +    KVM_RISCV_GET_TIMER(cs, frequency, reg);
> +
> +    return reg;
> +}
> +
>  static int kvm_riscv_get_regs_vector(CPUState *cs)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
> index 4bd98fddc776..58518988681d 100644
> --- a/target/riscv/kvm/kvm_riscv.h
> +++ b/target/riscv/kvm/kvm_riscv.h
> @@ -28,5 +28,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>  void riscv_kvm_aplic_request(void *opaque, int irq, int level);
>  int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>  void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs);
>
>  #endif
> --
> 2.17.1
>
>

Alistair Francis March 22, 2024, 4:47 a.m. UTC | #3

On Thu, Mar 14, 2024 at 4:17 PM Yong-Xuan Wang <[e-mail]> wrote:
>
> The timebase-frequency of guest OS should be the same with host
> machine. The timebase-frequency value in DTS should be got from
> hypervisor when using KVM acceleration.
>
> Reviewed-by: Andrew Jones <[e-mail]>
> Signed-off-by: Yong-Xuan Wang <[e-mail]>

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> ---
> Changelog
> v2:
> - update the function definition
> - restructure if-else statement
> ---
>  hw/riscv/virt.c              | 2 ++
>  target/riscv/kvm/kvm-cpu.c   | 9 +++++++++
>  target/riscv/kvm/kvm_riscv.h | 1 +
>  3 files changed, 12 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a094af97c32a..533b17799581 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -711,6 +711,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>
>      qemu_fdt_add_subnode(ms->fdt, "/cpus");
>      qemu_fdt_setprop_cell(ms->fdt, "/cpus", "timebase-frequency",
> +                          kvm_enabled() ?
> +                          kvm_riscv_get_timebase_frequency(first_cpu) :
>                            RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ);
>      qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#size-cells", 0x0);
>      qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#address-cells", 0x1);
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c7afdb1e81b7..bbb115eaa867 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -739,6 +739,15 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
>      env->kvm_timer_dirty = false;
>  }
>
> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs)
> +{
> +    uint64_t reg;
> +
> +    KVM_RISCV_GET_TIMER(cs, frequency, reg);
> +
> +    return reg;
> +}
> +
>  static int kvm_riscv_get_regs_vector(CPUState *cs)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
> index 4bd98fddc776..58518988681d 100644
> --- a/target/riscv/kvm/kvm_riscv.h
> +++ b/target/riscv/kvm/kvm_riscv.h
> @@ -28,5 +28,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>  void riscv_kvm_aplic_request(void *opaque, int irq, int level);
>  int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>  void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs);
>
>  #endif
> --
> 2.17.1
>
>

Michael Tokarev April 26, 2024, 11:44 p.m. UTC | #4

14.03.2024 09:15, Yong-Xuan Wang:
> The timebase-frequency of guest OS should be the same with host
> machine. The timebase-frequency value in DTS should be got from
> hypervisor when using KVM acceleration.

This change ended up in stable-8.2 (v8.2.3).  Interestingly, this thing
compiled not even once, or else it would be obvious it fails to compile.
Somehow I was too used to CI, forgetting that we don't have riscv *host*
in CI (and I don't have one locally either).  So 8.2.3 is broken on
riscv64 *host*.

In 8.2, KVM_RISCV_GET_TIMER macro accepts 4 arguments, because it does
not have 10f86d1b845087d1 "target/riscv/kvm: change timer regs size to u64".

What do you think, should I revert this change for stable-8.2, or pick
10f86d1b845087d1 too, or change this commit (fix timebase-frequency) to
provide the missing argument for this macro?

Thanks,

/mjt


> Reviewed-by: Andrew Jones <[e-mail]>
> Signed-off-by: Yong-Xuan Wang <[e-mail]>
> 
> ---
> Changelog
> v2:
> - update the function definition
> - restructure if-else statement
> ---
>   hw/riscv/virt.c              | 2 ++
>   target/riscv/kvm/kvm-cpu.c   | 9 +++++++++
>   target/riscv/kvm/kvm_riscv.h | 1 +
>   3 files changed, 12 insertions(+)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a094af97c32a..533b17799581 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -711,6 +711,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>   
>       qemu_fdt_add_subnode(ms->fdt, "/cpus");
>       qemu_fdt_setprop_cell(ms->fdt, "/cpus", "timebase-frequency",
> +                          kvm_enabled() ?
> +                          kvm_riscv_get_timebase_frequency(first_cpu) :
>                             RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ);
>       qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#size-cells", 0x0);
>       qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#address-cells", 0x1);
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c7afdb1e81b7..bbb115eaa867 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -739,6 +739,15 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
>       env->kvm_timer_dirty = false;
>   }
>   
> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs)
> +{
> +    uint64_t reg;
> +
> +    KVM_RISCV_GET_TIMER(cs, frequency, reg);
> +
> +    return reg;
> +}
> +
>   static int kvm_riscv_get_regs_vector(CPUState *cs)
>   {
>       RISCVCPU *cpu = RISCV_CPU(cs);
> diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
> index 4bd98fddc776..58518988681d 100644
> --- a/target/riscv/kvm/kvm_riscv.h
> +++ b/target/riscv/kvm/kvm_riscv.h
> @@ -28,5 +28,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>   void riscv_kvm_aplic_request(void *opaque, int irq, int level);
>   int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>   void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs);
>   
>   #endif

Andrew Jones April 27, 2024, 6:23 a.m. UTC | #5

On April 27, 2024 1:44:42 AM GMT+02:00, Michael Tokarev <[e-mail]> wrote:
>14.03.2024 09:15, Yong-Xuan Wang:
>> The timebase-frequency of guest OS should be the same with host
>> machine. The timebase-frequency value in DTS should be got from
>> hypervisor when using KVM acceleration.
>
>This change ended up in stable-8.2 (v8.2.3).  Interestingly, this thing
>compiled not even once, or else it would be obvious it fails to compile.
>Somehow I was too used to CI, forgetting that we don't have riscv *host*
>in CI (and I don't have one locally either).  So 8.2.3 is broken on
>riscv64 *host*.

It's possible to cross-compile qemu, so it'd be good to add that to the CI for riscv until we can add native compiling.

>
>In 8.2, KVM_RISCV_GET_TIMER macro accepts 4 arguments, because it does
>not have 10f86d1b845087d1 "target/riscv/kvm: change timer regs size to u64".
>
>What do you think, should I revert this change for stable-8.2, or pick
>10f86d1b845087d1 too, or change this commit (fix timebase-frequency) to
>provide the missing argument for this macro?

Changing the timer regs to u64 is an rv32 fix, so it's reasonable to also pick it up. I suggest we keep this patch one way or another, though.

Thanks,
drew

>
>Thanks,
>
>/mjt
>
>
>> Reviewed-by: Andrew Jones <[e-mail]>
>> Signed-off-by: Yong-Xuan Wang <[e-mail]>
>> 
>> ---
>> Changelog
>> v2:
>> - update the function definition
>> - restructure if-else statement
>> ---
>>   hw/riscv/virt.c              | 2 ++
>>   target/riscv/kvm/kvm-cpu.c   | 9 +++++++++
>>   target/riscv/kvm/kvm_riscv.h | 1 +
>>   3 files changed, 12 insertions(+)
>> 
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index a094af97c32a..533b17799581 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -711,6 +711,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>>         qemu_fdt_add_subnode(ms->fdt, "/cpus");
>>       qemu_fdt_setprop_cell(ms->fdt, "/cpus", "timebase-frequency",
>> +                          kvm_enabled() ?
>> +                          kvm_riscv_get_timebase_frequency(first_cpu) :
>>                             RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ);
>>       qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#size-cells", 0x0);
>>       qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#address-cells", 0x1);
>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>> index c7afdb1e81b7..bbb115eaa867 100644
>> --- a/target/riscv/kvm/kvm-cpu.c
>> +++ b/target/riscv/kvm/kvm-cpu.c
>> @@ -739,6 +739,15 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
>>       env->kvm_timer_dirty = false;
>>   }
>>   +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs)
>> +{
>> +    uint64_t reg;
>> +
>> +    KVM_RISCV_GET_TIMER(cs, frequency, reg);
>> +
>> +    return reg;
>> +}
>> +
>>   static int kvm_riscv_get_regs_vector(CPUState *cs)
>>   {
>>       RISCVCPU *cpu = RISCV_CPU(cs);
>> diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
>> index 4bd98fddc776..58518988681d 100644
>> --- a/target/riscv/kvm/kvm_riscv.h
>> +++ b/target/riscv/kvm/kvm_riscv.h
>> @@ -28,5 +28,6 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>>   void riscv_kvm_aplic_request(void *opaque, int irq, int level);
>>   int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
>>   void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
>> +uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs);
>>     #endif
>

Michael Tokarev April 27, 2024, 6:59 a.m. UTC | #6

27.04.2024 09:23, Andrew Jones wrote:
> On April 27, 2024 1:44:42 AM GMT+02:00, Michael Tokarev <[e-mail]> wrote:
>> 14.03.2024 09:15, Yong-Xuan Wang:
>>> The timebase-frequency of guest OS should be the same with host
>>> machine. The timebase-frequency value in DTS should be got from
>>> hypervisor when using KVM acceleration.
>>
>> This change ended up in stable-8.2 (v8.2.3).  Interestingly, this thing
>> compiled not even once, or else it would be obvious it fails to compile.
>> Somehow I was too used to CI, forgetting that we don't have riscv *host*
>> in CI (and I don't have one locally either).  So 8.2.3 is broken on
>> riscv64 *host*.
> 
> It's possible to cross-compile qemu, so it'd be good to add that to the CI for riscv until we can add native compiling.

Yes, definitely.  Qemu is already being cross-compiled on all "other"
architectures during CI.  But it is also being *run*, not just compiled.
And this is what's broken on riscv64 for almost a year now, and this
job has been disabled.  Instead, the *run* part of this job needs to
be disabled, but *build* part should be kept.

>> In 8.2, KVM_RISCV_GET_TIMER macro accepts 4 arguments, because it does
>> not have 10f86d1b845087d1 "target/riscv/kvm: change timer regs size to u64".
>>
>> What do you think, should I revert this change for stable-8.2, or pick
>> 10f86d1b845087d1 too, or change this commit (fix timebase-frequency) to
>> provide the missing argument for this macro?
> 
> Changing the timer regs to u64 is an rv32 fix, so it's reasonable to also pick it up. I suggest we keep this patch one way or another, though.

Okay, so I need help choosing which patches to pick.

10f86d1b845087d1 isn't sufficient, since it relies on 450bd6618fda3d
"target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64".  In the same series
there also was 49c211ffca00fdf7c "target/riscv/kvm: change KVM_REG_RISCV_FP_F
to u32" - is it also needed?

Please tell me the set of things I need for stable-8.2 here.  I'd
love to makes 8.2.4 release really soon, to fix this breakage.

Also, right now I don't know how to even compile-test it.  So meanwhile I'll
try to fix that and push this change to qemu master (to re-enable riscv64
CI job but only build part of it).  I don't have riscv hardware handy :)

Thanks,

/mjt

Michael Tokarev April 27, 2024, 7:24 a.m. UTC | #7

27.04.2024 09:59, Michael Tokarev wrote:
> 27.04.2024 09:23, Andrew Jones wrote:
...
>> It's possible to cross-compile qemu, so it'd be good to add that to the CI for riscv until we can add native compiling.
> 
> Yes, definitely.  Qemu is already being cross-compiled on all "other"
> architectures during CI.  But it is also being *run*, not just compiled.
> And this is what's broken on riscv64 for almost a year now, and this
> job has been disabled.  Instead, the *run* part of this job needs to
> be disabled, but *build* part should be kept.

Aha. I was wrong. And I was there before too, for sure, - just forgot
about it. In order to be cross-compiled, the cross-build environment
needs to have target -dev libraries, not only the cross-compiler.
And this is where debian riscv64 port is failing.

So no, it is not currently possible to cross-compile qemu at least
on debian without building whole cross-environment with all libraries
and other necessary stuff.

I'll try to use debian riscv64 porterbox to at least verify the new
set of patches we'll pick here to fix this breakage, at least compiles
on riscv64 :)

> 10f86d1b845087d1 isn't sufficient, since it relies on 450bd6618fda3d
> "target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64".  In the same series
> there also was 49c211ffca00fdf7c "target/riscv/kvm: change KVM_REG_RISCV_FP_F
> to u32" - is it also needed?

49c211ffca00fdf7c is also needed.  So it's 3 so far, still not compile-
tested.  Anything else?

/mjt

Andrew Jones April 27, 2024, 3:17 p.m. UTC | #8

On April 27, 2024 9:24:04 AM GMT+02:00, Michael Tokarev <[e-mail]> wrote:
>27.04.2024 09:59, Michael Tokarev wrote:
>> 27.04.2024 09:23, Andrew Jones wrote:
>...
>>> It's possible to cross-compile qemu, so it'd be good to add that to the CI for riscv until we can add native compiling.
>> 
>> Yes, definitely.  Qemu is already being cross-compiled on all "other"
>> architectures during CI.  But it is also being *run*, not just compiled.
>> And this is what's broken on riscv64 for almost a year now, and this
>> job has been disabled.  Instead, the *run* part of this job needs to
>> be disabled, but *build* part should be kept.
>
>Aha. I was wrong. And I was there before too, for sure, - just forgot
>about it. In order to be cross-compiled, the cross-build environment
>needs to have target -dev libraries, not only the cross-compiler.
>And this is where debian riscv64 port is failing.
>
>So no, it is not currently possible to cross-compile qemu at least
>on debian without building whole cross-environment with all libraries
>and other necessary stuff.
>
>I'll try to use debian riscv64 porterbox to at least verify the new
>set of patches we'll pick here to fix this breakage, at least compiles
>on riscv64 :)

I wrote instructions [2] for how to cross-compile without a full environment/container once. It might be better for quick, local testing.

[2] https://lore.kernel.org/qemu-riscv/[e-mail]/

>
>> 10f86d1b845087d1 isn't sufficient, since it relies on 450bd6618fda3d
>> "target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64".  In the same series
>> there also was 49c211ffca00fdf7c "target/riscv/kvm: change KVM_REG_RISCV_FP_F
>> to u32" - is it also needed?
>
>49c211ffca00fdf7c is also needed.  So it's 3 so far, still not compile-
>tested.  Anything else?

Those 3, the first of the series [1], are good. Not sure why it's still not compiling.

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01132.html

drew

Michael Tokarev April 27, 2024, 4:39 p.m. UTC | #9

27.04.2024 18:17, Andrew Jones :
> I wrote instructions [2] for how to cross-compile without a full environment/container once. It might be better for quick, local testing.
> 
> [2] https://lore.kernel.org/qemu-riscv/[e-mail]/

I just extracted a few packages from debian riscv64 (like libglib & deps)
in a separate dir and pointed various tools (pkgconf, gcc -I, gcc -L) to
that dir.


>> 49c211ffca00fdf7c is also needed.  So it's 3 so far, still not compile-
>> tested.  Anything else?
> 
> Those 3, the first of the series [1], are good. Not sure why it's still not compiling.

Yes, I picked up these 3 I mentioned, in addition to the problematic one
which is part of 8.2.3.  Once I had the build environment, I tried compiling
it, and it builds fine.  I wrote it is not compile-TESTED, not as it fails
to compile.

Also, I tried to build qemu on a real riscv64 hardware (on a debian porterbox),
-- it built fine (with the 3 mentioned changes applied) and is now running
tests, but it looks like it will be fine too.

> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01132.html

So yes, I'm picking these additional 3 from this set, - the ones which
I already mentioned.

Thanks,

/mjt

Alistair Francis April 29, 2024, 2:01 a.m. UTC | #10

On Sun, Apr 28, 2024 at 2:41 AM Michael Tokarev <[e-mail]> wrote:
>
> 27.04.2024 18:17, Andrew Jones :
> > I wrote instructions [2] for how to cross-compile without a full environment/container once. It might be better for quick, local testing.
> >
> > [2] https://lore.kernel.org/qemu-riscv/[e-mail]/
>
> I just extracted a few packages from debian riscv64 (like libglib & deps)
> in a separate dir and pointed various tools (pkgconf, gcc -I, gcc -L) to
> that dir.
>
>
> >> 49c211ffca00fdf7c is also needed.  So it's 3 so far, still not compile-
> >> tested.  Anything else?
> >
> > Those 3, the first of the series [1], are good. Not sure why it's still not compiling.
>
> Yes, I picked up these 3 I mentioned, in addition to the problematic one
> which is part of 8.2.3.  Once I had the build environment, I tried compiling
> it, and it builds fine.  I wrote it is not compile-TESTED, not as it fails
> to compile.
>
> Also, I tried to build qemu on a real riscv64 hardware (on a debian porterbox),
> -- it built fine (with the 3 mentioned changes applied) and is now running
> tests, but it looks like it will be fine too.
>
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01132.html
>
> So yes, I'm picking these additional 3 from this set, - the ones which
> I already mentioned.

Thanks so much for testing that.

I'm not really sure what to do about the RISC-V host builds. I keep
expecting distros to ship better RISC-V support, but still they don't
seem to have it

Alistair

>
> Thanks,
>
> /mjt
>
>
> --
> GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
> New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
> Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
> Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
>
>

Patch

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a094af97c32a..533b17799581 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -711,6 +711,8 @@  static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 
     qemu_fdt_add_subnode(ms->fdt, "/cpus");
     qemu_fdt_setprop_cell(ms->fdt, "/cpus", "timebase-frequency",
+                          kvm_enabled() ?
+                          kvm_riscv_get_timebase_frequency(first_cpu) :
                           RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ);
     qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#size-cells", 0x0);
     qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#address-cells", 0x1);
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index c7afdb1e81b7..bbb115eaa867 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -739,6 +739,15 @@  static void kvm_riscv_put_regs_timer(CPUState *cs)
     env->kvm_timer_dirty = false;
 }
 
+uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs)
+{
+    uint64_t reg;
+
+    KVM_RISCV_GET_TIMER(cs, frequency, reg);
+
+    return reg;
+}
+
 static int kvm_riscv_get_regs_vector(CPUState *cs)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
index 4bd98fddc776..58518988681d 100644
--- a/target/riscv/kvm/kvm_riscv.h
+++ b/target/riscv/kvm/kvm_riscv.h
@@ -28,5 +28,6 @@  void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
 void riscv_kvm_aplic_request(void *opaque, int irq, int level);
 int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
 void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
+uint64_t kvm_riscv_get_timebase_frequency(CPUState *cs);
 
 #endif