On 10.02.20 13:57, Cornelia Huck wrote:
> On Mon, 10 Feb 2020 13:26:35 +0100
> Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
>
>> On 08.02.20 15:57, Thomas Huth wrote:
>>> On 07/02/2020 12.39, Christian Borntraeger wrote:
>>>> From: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>>
>>>> Add documentation for KVM_CAP_S390_PROTECTED capability and the
>>>> KVM_S390_PV_COMMAND and KVM_S390_PV_COMMAND_VCPU ioctls.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>> [borntraeger@xxxxxxxxxx: patch merging, splitting, fixing]
>>>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>>>> ---
>>>> Documentation/virt/kvm/api.txt | 61 ++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 61 insertions(+)
>
>>>> +4.125 KVM_S390_PV_COMMAND
>>>> +
>>>> +Capability: KVM_CAP_S390_PROTECTED
>>>> +Architectures: s390
>>>> +Type: vm ioctl
>>>> +Parameters: struct kvm_pv_cmd
>>>> +Returns: 0 on success, < 0 on error
>>>> +
>>>> +struct kvm_pv_cmd {
>>>> + __u32 cmd; /* Command to be executed */
>>>> + __u16 rc; /* Ultravisor return code */
>>>> + __u16 rrc; /* Ultravisor return reason code */
>>>> + __u64 data; /* Data or address */
>>>
>>> That remindes me ... do we maybe want a "reserved" field in here for
>>> future extensions? Or is the "data" pointer enough?
>>
>>
>> This is now:
>>
>> struct kvm_pv_cmd {
>>
>> __u32 cmd; /* Command to be executed */
>> __u32 flags; /* flags for future extensions. Must be 0 for now */
>> __u64 data; /* Data or address */
>> __u64 reserved[2];
>> };
>
> Ok, that is where you add this... but still, the question: are those
> fields only ever set by userspace, or could the kernel return things in
> the reserved fields in the future?
I will change the IOWR to make sure that we can have both directions.
>
> Also, two 64 bit values seem a bit arbitrary... what about a data
> address + length construct instead? (Length might be a fixed value per
> flag?)
When you look at all the other examples we define those as reserved bytes
The idea is to have no semantics at all. Whenever we add a new flag we will
replace the reserved bytes with a new meaning.
e.g. see
struct kvm_s390_skeys {
__u64 start_gfn;
__u64 count;
__u64 skeydata_addr;
__u32 flags;
__u32 reserved[9];
};
or
/* for KVM_S390_MEM_OP and KVM_S390_SIDA_OP */
struct kvm_s390_mem_op {
/* in */
__u64 gaddr; /* the guest address */
__u64 flags; /* flags */
__u32 size; /* amount of bytes */
__u32 op; /* type of operation */
__u64 buf; /* buffer in userspace */
__u8 ar; /* the access register number */
__u8 reserved21[3]; /* should be set to 0 */
__u32 offset; /* offset into the sida */
__u8 reserved28[24]; /* should be set to 0 */
};
