Re: [PATCH v1 03/12] s390x/tcg: convert real to absolute address for RRBE, SSKE and ISKE

[David Hildenbrand]

On 06.08.21 13:25, Cornelia Huck wrote:
> On Fri, Aug 06 2021, David Hildenbrand <david@redhat.com> wrote:
>
> > On 06.08.21 09:11, Thomas Huth wrote:
> > > On 06/08/2021 08.52, David Hildenbrand wrote:
> > > > > According to the PoP:
> > > > >
> > > > > "When the enhanced-DAT facility 1 is not installed, or
> > > > >      when the facility is installed but the multiple-block
> > > > >      control is zero, general register R 2 contains a real
> > > > >      address. When the enhanced-DAT facility 1 is
> > > > >      installed and the multiple-block control is one, gen-
> > > > >      eral register R 2 contains an absolute address."
> > > > >
> > > > > Don't we have to take that into consideration here, too?
> > > >
> > > > We don't support EDAT1 a.k.a. huge pages yet. If we ever do, we have to
> > > > further extend this code.
> > >
> > > Ok, then maybe add a comment or assert() to make sure that we don't forget?
> >
> > Well, we'll need modifications and extensions all over the place to
> > support EDAT1, so I'm not sure this will really help ... we'll have to
> > carefully scan the PoP either way.
>
> Something like
> /* always real address, as long as we don't implement EDAT1 */
> would still be useful, I think.

I am not a friend of describing what to be done with additional CPU 
features. We have the PoP for that: just imagine you read an old version 
of the PoP, the code as is would make perfect sense even without ever 
knowing what EDAT1 is -- and you can verify that the code is correct.

For now I added to the patch description:

"
In the future, when adding EDAT1 support, we'll have to pay attention to
SSKE handling, as we'll be dealing with absolute addresses when the
multiple-block control is one.
"

--
Thanks,

David / dhildenb