On 05/02/2020 13.16, Janosch Frank wrote:
> On 2/5/20 1:02 PM, Thomas Huth wrote:
>> On 03/02/2020 14.19, Christian Borntraeger wrote:
>>> From: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>
>>> Now that we can't access guest memory anymore, we have a dedicated
>>> sattelite block that's a bounce buffer for instruction data.
>>>
>>> We re-use the memop interface to copy the instruction data to / from
>>> userspace. This lets us re-use a lot of QEMU code which used that
>>> interface to make logical guest memory accesses which are not possible
>>> anymore in protected mode anyway.
>>>
>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>> ---
>> [...]
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index eab741bc12c3..20969ce12096 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -466,7 +466,7 @@ struct kvm_translation {
>>> __u8 pad[5];
>>> };
>>>
>>> -/* for KVM_S390_MEM_OP */
>>> +/* for KVM_S390_MEM_OP and KVM_S390_SIDA_OP */
>>> struct kvm_s390_mem_op {
>>> /* in */
>>> __u64 gaddr; /* the guest address */
>>> @@ -475,11 +475,17 @@ struct kvm_s390_mem_op {
>>> __u32 op; /* type of operation */
>>> __u64 buf; /* buffer in userspace */
>>> __u8 ar; /* the access register number */
>>> - __u8 reserved[31]; /* should be set to 0 */
>>> + __u8 reserved21[3]; /* should be set to 0 */
>>> + __u32 offset; /* offset into the sida */
>>> + __u8 reserved28[24]; /* should be set to 0 */
>>> };
>>> +
>>> +
>>> /* types for kvm_s390_mem_op->op */
>>> #define KVM_S390_MEMOP_LOGICAL_READ 0
>>> #define KVM_S390_MEMOP_LOGICAL_WRITE 1
>>> +#define KVM_S390_MEMOP_SIDA_READ 2
>>> +#define KVM_S390_MEMOP_SIDA_WRITE 3
>>> /* flags for kvm_s390_mem_op->flags */
>>> #define KVM_S390_MEMOP_F_CHECK_ONLY (1ULL << 0)
>>> #define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
>>> @@ -1510,6 +1516,7 @@ struct kvm_pv_cmd {
>>> /* Available with KVM_CAP_S390_PROTECTED */
>>> #define KVM_S390_PV_COMMAND _IOW(KVMIO, 0xc5, struct kvm_pv_cmd)
>>> #define KVM_S390_PV_COMMAND_VCPU _IOW(KVMIO, 0xc6, struct kvm_pv_cmd)
>>> +#define KVM_S390_SIDA_OP _IOW(KVMIO, 0xc7, struct kvm_s390_mem_op)
>>>
>>> /* Secure Encrypted Virtualization command */
>>> enum sev_cmd_id {
>>>
>>
>> Uh, why the mix of a new ioctl with the existing mem_op stuff? Could you
>> please either properly integrate this into the MEM_OP ioctl (and e.g.
>> use gaddr as offset for the new SIDA_READ and SIDA_WRITE subcodes), or
>> completely separate it for a new ioctl, i.e. introduce a new struct for
>> the new ioctl instead of recycling the struct kvm_s390_mem_op here?
>> (and in case you ask me, I'd slightly prefer to integrate everything
>> into MEM_OP instead of introducing a new ioctl here).
>
> *cough* David and Christian didn't like the memop solution and it took
> me a long time to get this to work properly in QEMU...
I also don't like to re-use MEMOP_LOGICAL_READ and MEMOP_LOGICAL_WRITE
for the SIDA like you've had it in RFC v1 ... but what's wrong with
using KVM_S390_MEMOP_SIDA_READ and KVM_S390_MEMOP_SIDA_WRITE with the
MEM_OP ioctl directly?
Thomas
Attachment:
signature.asc
Description: OpenPGP digital signature
