On 04/07/2017 01:42 PM, David Hildenbrand wrote: > >> + >> + hpa = scb_s->sdnxo; >> + if (hpa) { >> + gpa = scb_o->sdnxo; > > Other blocks in this function explicitly extract the gpa portion > > gpa = scb_o->sdnxo & ~0xfUL; > > But this shouldn't matter as it will be dropped either way when > converting to pfn. At least for now this should work. Yes, this should not matter as we unpin it on a page base anyway. > >> + unpin_guest_page(vcpu->kvm, gpa, hpa); >> + scb_s->sdnxo = 0; >> + } >> } >> >> /* >> @@ -590,6 +602,33 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> goto unpin; >> scb_s->riccbd = hpa; >> } >> + if ((scb_s->ecb & ECB_GS) && !(scb_s->ecd & ECD_HOSTREGMGMT)) { >> + unsigned long sdnxc; >> + >> + gpa = scb_o->sdnxo & ~0xfUL; >> + sdnxc = scb_o->sdnxo & 0xfUL; >> + if (!gpa || !(gpa & ~0x1fffUL)) { >> + rc = set_validity_icpt(scb_s, 0x10b0U); >> + goto unpin; >> + } >> + if (sdnxc < 6 || sdnxc > 12) { >> + rc = set_validity_icpt(scb_s, 0x10b1U); >> + goto unpin; >> + } >> + if (gpa & ((1 << sdnxc) - 1)) { >> + rc = set_validity_icpt(scb_s, 0x10b2U); >> + goto unpin; >> + } >> + /* Due to alignment rules (checked above) this cannot >> + * cross page boundaries >> + */ >> + rc = pin_guest_page(vcpu->kvm, gpa, &hpa); >> + if (rc == -EINVAL) >> + rc = set_validity_icpt(scb_s, 0x10b0U); >> + if (rc) >> + goto unpin; >> + scb_s->sdnxo = hpa; > > I'm curious, this will result in no sdnxc getting set. Shouldn't there > be something like the following, cause otherwise there would be a > validity icpt 0x10b1U (guessing from the code above). > > scb_s->sdnxo = hpa | sdnxc; Indeed this looks like a bug (and the right fix). This was not noticed during test since KVM enables the host management of registers and we do not use the sdnx. So for the KVM under KVM case this does not matter. Still, we should fix it. Radim, Paolo, do you want a respin of the whole series or can I simply send a fixup patch with the next round of patches? > >> + } >> return 0; >> unpin: >> unpin_blocks(vcpu, vsie_page); >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index f51d508..c9d5227 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -883,6 +883,7 @@ struct kvm_ppc_resize_hpt { >> #define KVM_CAP_PPC_MMU_RADIX 134 >> #define KVM_CAP_PPC_MMU_HASH_V3 135 >> #define KVM_CAP_IMMEDIATE_EXIT 136 >> +#define KVM_CAP_S390_GS 137 >> >> #ifdef KVM_CAP_IRQ_ROUTING >> >> > >