Re: [PATCH v1 2/3] KVM: s390: add stat counter for shadow gmap events

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Tue, 09 May 2023 16:53:13 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Quoting Claudio Imbrenda (2023-05-09 13:43:51)
> [...]
> > > diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> > > index 3eb85f254881..8348a0095f3a 100644
> > > --- a/arch/s390/kvm/gaccess.c
> > > +++ b/arch/s390/kvm/gaccess.c
> > > @@ -1382,6 +1382,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> > >                                 unsigned long *pgt, int *dat_protection,
> > >                                 int *fake)
> > >  {
> > > +     struct kvm *kvm;
> > >       struct gmap *parent;
> > >       union asce asce;
> > >       union vaddress vaddr;
> > > @@ -1390,6 +1391,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> > >  
> > >       *fake = 0;
> > >       *dat_protection = 0;
> > > +     kvm = sg->private;
> > >       parent = sg->parent;
> > >       vaddr.addr = saddr;
> > >       asce.val = sg->orig_asce;
> > > @@ -1450,6 +1452,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> > >               rc = gmap_shadow_r2t(sg, saddr, rfte.val, *fake);
> > >               if (rc)
> > >                       return rc;
> > > +             kvm->stat.gmap_shadow_r2++;
> > >       }
> > >               fallthrough;
> > >       case ASCE_TYPE_REGION2: {
> > > @@ -1478,6 +1481,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> > >               rc = gmap_shadow_r3t(sg, saddr, rste.val, *fake);
> > >               if (rc)
> > >                       return rc;
> > > +             kvm->stat.gmap_shadow_r3++;
> > >       }
> > >               fallthrough;
> > >       case ASCE_TYPE_REGION3: {
> > > @@ -1515,6 +1519,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> > >               rc = gmap_shadow_sgt(sg, saddr, rtte.val, *fake);
> > >               if (rc)
> > >                       return rc;
> > > +             kvm->stat.gmap_shadow_segment++;
> > >       }
> > >               fallthrough;
> > >       case ASCE_TYPE_SEGMENT: {
> > > @@ -1548,6 +1553,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> > >               rc = gmap_shadow_pgt(sg, saddr, ste.val, *fake);
> > >               if (rc)
> > >                       return rc;
> > > +             kvm->stat.gmap_shadow_page++;  
> > 
> > do I understand correctly that, if several levels need to be shadowed
> > at the same time, you will increment every affected counter, and not
> > just the highest or lowest level?
> > 
> > if so, please add a brief explanation to the patch description  
> 
> Yes, that seemed like the simplest thing to do.
> 
> Will add a explanation.
> 
> Or should I add a flag and only increment the top level?

that's up to you, see what makes more sense