On Wed, 5 Feb 2020 09:54:23 +0100
Michael Mueller <mimu@xxxxxxxxxxxxx> wrote:
> On 05.02.20 07:59, Thomas Huth wrote:
> > On 03/02/2020 14.19, Christian Borntraeger wrote:
> >> From: Michael Mueller <mimu@xxxxxxxxxxxxx>
> >>
> >> Define the interruption injection codes and the related fields in the
> >> sie control block for PVM interruption injection.
> >
> > You seem to only add the details for external interrupts and I/O
> > interrupts here? Maybe mention this in the description ... otherwise it
> > is confusing when you read patch 17 later ... or maybe merge this patch
> > here with patch 17 ?
>
> In that case I will merge patch 17 into this one.
Merging patch 17 sounds good to me, as it gets us all of the new
interrupt controls in one go.
>
> >
> >> Signed-off-by: Michael Mueller <mimu@xxxxxxxxxxxxx>
> >> ---
> >> arch/s390/include/asm/kvm_host.h | 25 +++++++++++++++++++++----
> >> 1 file changed, 21 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> >> index 58845b315be0..a45d10d87a8a 100644
> >> --- a/arch/s390/include/asm/kvm_host.h
> >> +++ b/arch/s390/include/asm/kvm_host.h
> >> @@ -222,7 +222,15 @@ struct kvm_s390_sie_block {
> >> __u8 icptcode; /* 0x0050 */
> >> __u8 icptstatus; /* 0x0051 */
> >> __u16 ihcpu; /* 0x0052 */
> >> - __u8 reserved54[2]; /* 0x0054 */
> >> + __u8 reserved54; /* 0x0054 */
> >> +#define IICTL_CODE_NONE 0x00
> >> +#define IICTL_CODE_MCHK 0x01
> >> +#define IICTL_CODE_EXT 0x02
> >> +#define IICTL_CODE_IO 0x03
> >> +#define IICTL_CODE_RESTART 0x04
> >> +#define IICTL_CODE_SPECIFICATION 0x10
> >> +#define IICTL_CODE_OPERAND 0x11
> >> + __u8 iictl; /* 0x0055 */
> >> __u16 ipa; /* 0x0056 */
> >> __u32 ipb; /* 0x0058 */
> >> __u32 scaoh; /* 0x005c */
> >> @@ -259,7 +267,8 @@ struct kvm_s390_sie_block {
> >> #define HPID_KVM 0x4
> >> #define HPID_VSIE 0x5
> >> __u8 hpid; /* 0x00b8 */
> >> - __u8 reservedb9[11]; /* 0x00b9 */
> >> + __u8 reservedb9[7]; /* 0x00b9 */
> >> + __u32 eiparams; /* 0x00c0 */
> >> __u16 extcpuaddr; /* 0x00c4 */
> >> __u16 eic; /* 0x00c6 */
> >> __u32 reservedc8; /* 0x00c8 */
> >> @@ -275,8 +284,16 @@ struct kvm_s390_sie_block {
> >> __u8 oai; /* 0x00e2 */
> >> __u8 armid; /* 0x00e3 */
> >> __u8 reservede4[4]; /* 0x00e4 */
> >> - __u64 tecmc; /* 0x00e8 */
> >> - __u8 reservedf0[12]; /* 0x00f0 */
> >> + union {
> >> + __u64 tecmc; /* 0x00e8 */
> >> + struct {
> >> + __u16 subchannel_id; /* 0x00e8 */
> >> + __u16 subchannel_nr; /* 0x00ea */
> >> + __u32 io_int_parm; /* 0x00ec */
> >> + __u32 io_int_word; /* 0x00f0 */
> >> + };
> >> + } __packed;
> >> + __u8 reservedf4[8]; /* 0x00f4 */
> >
> > Maybe add a comment to the new struct for which injection type it is
> > good for ... otherwise this might get hard to understand in the future
> > (especially if more stuff gets added like in patch 17).
>
> Up to know we don't have comments in the the SIE control block struct at
> all as this is part of the documentation, also for protected virtualization.
FWIW, I find the sie control block structure already hard to understand
right now. Some documentation would be useful, if possible.
>
> Any other opinions on that are welcome.
>
> >
> > Anyway,
> > Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
>
> thanks
>
> >
>
> Michael
>
