Re: [PATCH 09/35] KVM: s390: protvirt: Add KVM api documentation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 */
};











[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux