Re: [PATCH v3 2/4] KVM: Add unified KVM_GET/SET_VCPU_STATE IOCTL

[Jan Kiszka <jan.kiszka@siemens.com>]

Avi Kivity wrote:
> On 11/02/2009 06:20 PM, Jan Kiszka wrote:
>> Add a new IOCTL pair to retrieve or set the VCPU state in one chunk.
>> More precisely, the IOCTL is able to process a list of substates to be
>> read or written. This list is easily extensible without breaking the
>> existing ABI, thus we will no longer have to add new IOCTLs when we
>> discover a missing VCPU state field or want to support new hardware
>> features.
>>
>> This patch establishes the generic infrastructure for KVM_GET/
>> SET_VCPU_STATE, introduces KVM_CHECK_VCPU_STATES to query supported
>> substates, and adds support for the generic substates REGS, SREGS, FPU,
>> and MP. To avoid code duplication, the entry point for the corresponding
>> original IOCTLs are converted to make use of the new infrastructure
>> internally, too.
>>
>> +
>> +struct kvm_vcpu_substate {
>> +	__u32 type;	/* KVM_VCPU_STATE_* or KVM_$(ARCH)_VCPU_STATE_* */
>> +	__u32 pad;
>> +	__s64 offset;	/* payload offset to kvm_vcpu_state in bytes */
>> +};
>>    
> 
> 64-bit offset is a bit too much, and can't be made to work on 32-bit 
> hosts.  Also, why signed?  it's begging for trouble.

Please elaborate what troubles you see precisely. The offset is supposed
to make the whole user's address space reachable relative to the
header's base address. Therefore 64 bit, therefore signed.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux