linux-kernel - Re: [PATCH v1 2/2] s390/kvm: Move bitfields for dat tables

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240712095656.9604-A-hca@linux.ibm.com>
Date: Fri, 12 Jul 2024 11:56:56 +0200
From: Heiko Carstens <hca@...ux.ibm.com>
To: Claudio Imbrenda <imbrenda@...ux.ibm.com>
Cc: Alexander Gordeev <agordeev@...ux.ibm.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, linux-s390@...r.kernel.org, svens@...ux.ibm.com,
        gor@...ux.ibm.com, nrb@...ux.ibm.com, nsg@...ux.ibm.com,
        seiden@...ux.ibm.com, frankja@...ux.ibm.com, borntraeger@...ibm.com,
        gerald.schaefer@...ux.ibm.com, david@...hat.com
Subject: Re: [PATCH v1 2/2] s390/kvm: Move bitfields for dat tables

On Thu, Jul 11, 2024 at 06:23:48PM +0200, Claudio Imbrenda wrote:
> On Thu, 11 Jul 2024 17:16:23 +0200
> Alexander Gordeev <agordeev@...ux.ibm.com> wrote:
> > Hi Claudio,
> > 
> > > Once in a separate header, the structs become available everywhere. One
> > > possible usecase is to merge them in the s390 
> > > definitions, which is left as an exercise for the reader.  
> > 
> > Is my understanding correct that you potentially see page_table_entry::val /
> > region?_table_entry.*::val / segment_table_entry.* merged with pte_t::pte /
> > p?d_t::p?d?
> 
> that depends on how you want to do the merge
> 
> you could do:
> 
> typedef union {
> 	unsigned long pte;
> 	union page_table_entry hw;
> 	union page_table_entry_softbits sw;
> } pte_t;
> 
> then you would have pte_t::pte and pte_t::hw::val; unfortunately it's
> not possible to anonymously merge a named type.. 
> 
> this would be great but can't be done*:
> 
> typedef union {
> 	unsigned long pte;
> 	union page_table_entry;
> } pte_t;
> 
> [*] gcc actually supports it with an additional feature switch, but
> it's not standard C and I seriously doubt we should even think about
> doing it
> 
> another possibility is a plain
> 
> typedef union page_table_entry pte_t;
> 
> and then fix pte_val() and similar, but then you won't have the
> softbits.
> 
> 
> in the end, it's up to you how you want to merge them. I will
> have my own unions that I will use only inside KVM, that's enough for
> me.

We discussed this, and I really don't think we want this for the software
defined bits. In general I do like the idea of having bit fields and let
gcc do the decoding. But we have so many masks we use every for ptes and
friends that a single assignment won't work in most cases. So we eiter have
to do several assignments (which sometimes is a no-go when block concurrent
updates are required), or we have to stay with using both the existing
defines plus the new unions - which makes things even more complicated.

There might be uses cases where the hardware structures are also useful for
s390 core code, but I don't think we should go the route outlined above.