On 23.08.2018 13:53, Janosch Frank wrote: > On 8/23/18 1:47 PM, Pierre Morel wrote: >> On 23/08/2018 13:33, Janosch Frank wrote: >>> On 8/23/18 1:21 PM, David Hildenbrand wrote: >>>> On 23.08.2018 13:05, Janosch Frank wrote: >>>>> On 8/23/18 12:25 PM, Pierre Morel wrote: >>>>>> The comment preceding the shadow_crycb function is >>>>>> misleading, we effectively accept FORMAT2 CRYCB in the >>>>>> guest. >>>>> >>>>> I beg to differ: >>>>> >>>>> if (!(crycbd_o & vcpu->arch.sie_block->crycbd & CRYCB_FORMAT1)) >>>>> return 0; >>>> >>>> FORMAT2 includes bit FORMAT1 (backwards compatible) >>> >>> Right, this check is very misleading because of the constant, we >>> effectively test against Format 0 and Format 2. >>> >>> Can we make this clearer by explicitly ANDing 0x01 or adding a comment? >> >> yes, done, I modified the comment in front of the function. > > Which is not what I want, what I want is: > > /* CRYCB_FORMAT2 includes the bit for CRYCB_FORMAT1, so we allow both > formats here */ > if (!(crycbd_o & vcpu->arch.sie_block->crycbd & CRYCB_FORMAT1)) > return 0; While it's not wrong, it is also not required. And it might soon be obsolete again (with APXA, as you said, there we always have to check). But I'll leave that to you -- Thanks, David / dhildenb