1. Patchew
  2. linux
  3. [PATCH 0/3] KVM: s390: Couple of small cmpxchg() optimizations
  4. View patch

[PATCH 3/3] KVM: s390: Increase size of union sca_utility to four bytes

Heiko Carstens posted 3 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH 3/3] KVM: s390: Increase size of union sca_utility to four bytes
Posted by Heiko Carstens 1 year, 2 months ago 
kvm_s390_update_topology_change_report() modifies a single bit within
sca_utility using cmpxchg(). Given that the size of the sca_utility union
is two bytes this generates very inefficient code. Change the size to four
bytes, so better code can be generated.

Even though the size of sca_utility doesn't reflect architecture anymore
this seems to be the easiest and most pragmatic approach to avoid
inefficient code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 1cd8eaebd3c0..1cb1de232b9e 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -95,10 +95,10 @@ union ipte_control {
 };
 
 union sca_utility {
-	__u16 val;
+	__u32 val;
 	struct {
-		__u16 mtcr : 1;
-		__u16 reserved : 15;
+		__u32 mtcr : 1;
+		__u32	   : 31;
 	};
 };
 
@@ -107,7 +107,7 @@ struct bsca_block {
 	__u64	reserved[5];
 	__u64	mcn;
 	union sca_utility utility;
-	__u8	reserved2[6];
+	__u8	reserved2[4];
 	struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS];
 };
 
@@ -115,7 +115,7 @@ struct esca_block {
 	union ipte_control ipte_control;
 	__u64   reserved1[6];
 	union sca_utility utility;
-	__u8	reserved2[6];
+	__u8	reserved2[4];
 	__u64   mcn[4];
 	__u64   reserved3[20];
 	struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];
-- 
2.45.2
Re: [PATCH 3/3] KVM: s390: Increase size of union sca_utility to four bytes
Posted by Claudio Imbrenda 1 year, 2 months ago 
On Mon, 25 Nov 2024 12:50:39 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> kvm_s390_update_topology_change_report() modifies a single bit within
> sca_utility using cmpxchg(). Given that the size of the sca_utility union
> is two bytes this generates very inefficient code. Change the size to four
> bytes, so better code can be generated.
> 
> Even though the size of sca_utility doesn't reflect architecture anymore
> this seems to be the easiest and most pragmatic approach to avoid
> inefficient code.

wouldn't an atomic bit_op be better in that case?

> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 1cd8eaebd3c0..1cb1de232b9e 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -95,10 +95,10 @@ union ipte_control {
>  };
>  
>  union sca_utility {
> -	__u16 val;
> +	__u32 val;
>  	struct {
> -		__u16 mtcr : 1;
> -		__u16 reserved : 15;
> +		__u32 mtcr : 1;
> +		__u32	   : 31;
>  	};
>  };
>  
> @@ -107,7 +107,7 @@ struct bsca_block {
>  	__u64	reserved[5];
>  	__u64	mcn;
>  	union sca_utility utility;
> -	__u8	reserved2[6];
> +	__u8	reserved2[4];
>  	struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS];
>  };
>  
> @@ -115,7 +115,7 @@ struct esca_block {
>  	union ipte_control ipte_control;
>  	__u64   reserved1[6];
>  	union sca_utility utility;
> -	__u8	reserved2[6];
> +	__u8	reserved2[4];
>  	__u64   mcn[4];
>  	__u64   reserved3[20];
>  	struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];
Re: [PATCH 3/3] KVM: s390: Increase size of union sca_utility to four bytes
Posted by Heiko Carstens 1 year, 2 months ago 
On Mon, Nov 25, 2024 at 01:20:42PM +0100, Claudio Imbrenda wrote:
> On Mon, 25 Nov 2024 12:50:39 +0100
> Heiko Carstens <hca@linux.ibm.com> wrote:
> 
> > kvm_s390_update_topology_change_report() modifies a single bit within
> > sca_utility using cmpxchg(). Given that the size of the sca_utility union
> > is two bytes this generates very inefficient code. Change the size to four
> > bytes, so better code can be generated.
> > 
> > Even though the size of sca_utility doesn't reflect architecture anymore
> > this seems to be the easiest and most pragmatic approach to avoid
> > inefficient code.
> 
> wouldn't an atomic bit_op be better in that case?

I had that, but decided against it, since the generated code isn't shorter.
And it would require and unsigned long type within the union, or a cast,
which I also both disliked.
Re: [PATCH 3/3] KVM: s390: Increase size of union sca_utility to four bytes
Posted by Claudio Imbrenda 1 year, 2 months ago 
On Mon, 25 Nov 2024 14:40:22 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> On Mon, Nov 25, 2024 at 01:20:42PM +0100, Claudio Imbrenda wrote:
> > On Mon, 25 Nov 2024 12:50:39 +0100
> > Heiko Carstens <hca@linux.ibm.com> wrote:
> >   
> > > kvm_s390_update_topology_change_report() modifies a single bit within
> > > sca_utility using cmpxchg(). Given that the size of the sca_utility union
> > > is two bytes this generates very inefficient code. Change the size to four
> > > bytes, so better code can be generated.
> > > 
> > > Even though the size of sca_utility doesn't reflect architecture anymore
> > > this seems to be the easiest and most pragmatic approach to avoid
> > > inefficient code.  
> > 
> > wouldn't an atomic bit_op be better in that case?  
> 
> I had that, but decided against it, since the generated code isn't shorter.
> And it would require and unsigned long type within the union, or a cast,
> which I also both disliked.

fair enough

Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.