[RFC,v6,10/13] target/s390x: use kvm_enabled() to wrap call to kvm_s390_get_hpage_1m

Commit Message

Al Cho June 29, 2021, 2:19 p.m. UTC

this will allow to remove the kvm stubs.

Signed-off-by: Claudio Fontana <[e-mail]>
Signed-off-by: Cho, Yu-Chen <[e-mail]>
---
 target/s390x/diag.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Cornelia Huck June 30, 2021, 3:21 p.m. UTC | #1

On Tue, Jun 29 2021, "Cho, Yu-Chen" <[e-mail]> wrote:

> this will allow to remove the kvm stubs.
>
> Signed-off-by: Claudio Fontana <[e-mail]>
> Signed-off-by: Cho, Yu-Chen <[e-mail]>
> ---
>  target/s390x/diag.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index c17a2498a7..8405f69df0 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -20,6 +20,7 @@
>  #include "hw/s390x/ipl.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
>  #include "hw/s390x/pv.h"
> +#include "sysemu/kvm.h"
>  #include "kvm_s390x.h"
>  
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
> @@ -168,7 +169,7 @@ out:
>              return;
>          }
>  
> -        if (kvm_s390_get_hpage_1m()) {
> +        if (kvm_enabled() && kvm_s390_get_hpage_1m()) {

I think I asked before whether we should introduce a
s390_huge_page_backing() wrapper (which might be overkill)... any
opinions on that? I'm not really opposed to this patch here, either.

>              error_report("Protected VMs can currently not be backed with "
>                           "huge pages");
>              env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;

Al Cho July 1, 2021, 5:25 a.m. UTC | #2

Hi Cornelia,

Sorry for missing the reply.
I think it may not be worth it, as you said it seem to be the only call site for kvm_s390_get_hpage_1m().
So I think we could keep it.

Thanks,
            AL

Thomas Huth July 1, 2021, 7:16 p.m. UTC | #3

On 29/06/2021 16.19, Cho, Yu-Chen wrote:
> this will allow to remove the kvm stubs.
> 
> Signed-off-by: Claudio Fontana <[e-mail]>
> Signed-off-by: Cho, Yu-Chen <[e-mail]>
> ---
>   target/s390x/diag.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index c17a2498a7..8405f69df0 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -20,6 +20,7 @@
>   #include "hw/s390x/ipl.h"
>   #include "hw/s390x/s390-virtio-ccw.h"
>   #include "hw/s390x/pv.h"
> +#include "sysemu/kvm.h"
>   #include "kvm_s390x.h"
>   
>   int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
> @@ -168,7 +169,7 @@ out:
>               return;
>           }
>   
> -        if (kvm_s390_get_hpage_1m()) {
> +        if (kvm_enabled() && kvm_s390_get_hpage_1m()) {
>               error_report("Protected VMs can currently not be backed with "
>                            "huge pages");
>               env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;

Reviewed-by: Thomas Huth <[e-mail]>

Patch

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index c17a2498a7..8405f69df0 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -20,6 +20,7 @@ 
 #include "hw/s390x/ipl.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 #include "hw/s390x/pv.h"
+#include "sysemu/kvm.h"
 #include "kvm_s390x.h"
 
 int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
@@ -168,7 +169,7 @@  out:
             return;
         }
 
-        if (kvm_s390_get_hpage_1m()) {
+        if (kvm_enabled() && kvm_s390_get_hpage_1m()) {
             error_report("Protected VMs can currently not be backed with "
                          "huge pages");
             env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;