Re: [kvm-unit-tests PATCH v3 2/6] s390x: add function to set DAT mode for all interrupts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Janosch Frank (2023-06-05 10:42:40)
> On 6/1/23 09:01, Nico Boehr wrote:
> > When toggling DAT or switch address space modes, it is likely that
> 
> s/switch/switching/

Done 

[...]
> > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> > index 3f993a363ae2..d97b5a3a7e97 100644
> > --- a/lib/s390x/interrupt.c
> > +++ b/lib/s390x/interrupt.c
> > @@ -9,6 +9,7 @@
> >    */
> >   #include <libcflat.h>
> >   #include <asm/barrier.h>
> > +#include <asm/mem.h>
> >   #include <asm/asm-offsets.h>
> >   #include <sclp.h>
> >   #include <interrupt.h>
> > @@ -104,6 +105,40 @@ void register_ext_cleanup_func(void (*f)(struct stack_frame_int *))
> >       THIS_CPU->ext_cleanup_func = f;
> >   }
> >   
> > +/**
> > + * irq_set_dat_mode - Set the DAT mode of all interrupt handlers, except for
> > + * restart.
> > + * This will update the DAT mode and address space mode of all interrupt new
> > + * PSWs.
> > + *
> > + * Since enabling DAT needs initalized CRs and the restart new PSW is often used
> 
> s/initalized/initialized/

fixed

> > +void irq_set_dat_mode(bool dat, uint64_t as)
> > +{
> > +     struct psw* irq_psws[] = {
> > +             OPAQUE_PTR(GEN_LC_EXT_NEW_PSW),
> > +             OPAQUE_PTR(GEN_LC_SVC_NEW_PSW),
> > +             OPAQUE_PTR(GEN_LC_PGM_NEW_PSW),
> > +             OPAQUE_PTR(GEN_LC_MCCK_NEW_PSW),
> > +             OPAQUE_PTR(GEN_LC_IO_NEW_PSW),
> > +             NULL /* sentinel */
> > +     };
> > +
> > +     assert(as == AS_PRIM || as == AS_ACCR || as == AS_SECN || as == AS_HOME);
> 
> /* There are only 4 spaces */
> assert(as < 4); ?

Well you won't find that when when grepping for any of the AS_*, so no I don't
think that's better.

> > +
> > +     for (struct psw *psw = irq_psws[0]; psw != NULL; psw++) {
> 
> While this is ok in gnu99/c99 I generally prefer declaring the variable 
> outside of the loop since it's more readable when using structs and unions.

OK.

> 
> > +             psw->dat = dat;
> > +             if (dat)
> > +                     psw->as = as;
> 
> Does that check even matter?

Well, when DAT is off, AS bacially becomes a don't care. I thought it's a good
idea to preserve the AS in this case, but this can certainly be argued about.

I updated the comment and clarified that AS is only touched if DAT is on. Let me
know if you prefer removal of the if.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux