* [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-12-03 13:50 ` Andrew Jones
2018-11-28 14:45 ` [PATCH 02/12] KVM: arm/arm64: Factor out hypercall handling from PSCI code Steven Price
` (12 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
We introduce a paravirtualization interface for KVM/arm64 based on the
"Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
User space can specify a reserved area of memory for the guest and
inform KVM to populate the memory with information on stolen time and
Live Physical Time (LPT) that can be used to derive a stable
counter/timer for a guest subject to migration between hosts with
different counter frequencies.
A hypercall interface is provided for the guest to interrogate the
hypervisor's support for this interface and the location of the shared
memory structures.
Signed-off-by: Steven Price <steven.price@arm.com>
---
Documentation/virtual/kvm/arm/pvtime.txt | 169 +++++++++++++++++++++++
1 file changed, 169 insertions(+)
create mode 100644 Documentation/virtual/kvm/arm/pvtime.txt
diff --git a/Documentation/virtual/kvm/arm/pvtime.txt b/Documentation/virtual/kvm/arm/pvtime.txt
new file mode 100644
index 000000000000..1870b904075b
--- /dev/null
+++ b/Documentation/virtual/kvm/arm/pvtime.txt
@@ -0,0 +1,169 @@
+Paravirtualized time support for arm64
+======================================
+
+Arm specification DEN0057/A defined a standard for paravirtualised time
+support for Aarch64 guests:
+
+https://developer.arm.com/docs/den0057/a
+
+KVM/Arm64 implements this specification by providing some hypervisor service
+calls to support a paravirtualized guest obtaining a view of the amount of
+time stolen from its execution and a concept of Live Physical Time (LPT) which
+represents time during which the guest is running and works across migrations.
+
+Three new SMCCC compatible hypercalls are defined:
+
+PV_FEATURES 0xC5000020
+PV_TIME_LPT 0xC5000021
+PV_TIME_ST 0xC5000022
+
+These are only available in the SMC64/HVC64 calling convention as
+paravirtualized time is not available to 32 bit Arm guests.
+
+PV_FEATURES
+ Function ID: (uint32) : 0xC5000020
+ PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
+ Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
+ PV-time feature is supported by the hypervisor.
+
+PV_TIME_LPT
+ Function ID: (uint32) : 0xC5000021
+ Flags: (uint32) : Bit[0]: Request migration interrupts
+ (not currently supported by KVM)
+ Return value: (int64) : IPA of the shared live physical time data
+ structure or negative error code on failure:
+ NOT_SUPPORTED (-1)
+ INVALID_PARAMETERS (-2)
+
+PV_TIME_ST
+ Function ID: (uint32) : 0xC5000022
+ Return value: (int64) : IPA of the stolen time data structure for this
+ (V)CPU. On failure:
+ NOT_SUPPORTED (-1)
+
+Live Physical Time
+------------------
+
+The structure pointed to by the PV_TIME_LPT hypercall is as follows:
+
+ Field | Byte Length | Byte Offset | Description
+ --------------- | ----------- | ----------- | -------------------------------
+ Revision | 4 | 0 | Must be 0 for this revision
+ Attributes | 4 | 4 | Must be 0
+ sequence_number | 8 | 8 | Bit 0: reserved
+ | | | Bits 1:63 number of migrations
+ scale_mult | 8 | 16 | Multiplier to scale from native
+ | | | frequency to PV frequency
+ shift | 4 | 24 | Shift applied before multiplier
+ Reserved | 4 | 28 | Must be 0
+ Fn | 8 | 32 | Native frequency
+ Fpv | 8 | 40 | Paravirtualized frequency seen
+ | | | by guest
+ div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
+ | | | divide by Fpv
+
+Where scale_mult is defined as 2^(64-shift) * Fpv / Fn
+
+The structure will be updated by the hypervisor whenever the guest is migrated
+to a new host. It will be present within a reserved region of the normal
+memory given to the guest. The guest should not attempt to write into this
+memory.
+
+Stolen Time
+-----------
+
+The structure pointed to by the PV_TIME_ST hypercall is as follows:
+
+ Field | Byte Length | Byte Offset | Description
+ ----------- | ----------- | ----------- | --------------------------
+ Revision | 4 | 0 | Must be 0 for version 0.1
+ Attributes | 4 | 4 | Must be 0
+ Stolen time | 8 | 8 | Stolen time in unsigned
+ | | | nanoseconds indicating how
+ | | | much time this VCPU thread
+ | | | was involuntarily not
+ | | | running on a physical CPU.
+
+The structure will be updated by the hypervisor periodically as time is stolen
+from the VCPU. It will be present within a reserved region of the normal
+memory given to the guest. The guest should not attempt to write into this
+memory. There is a structure by VCPU of the guest.
+
+User space interface
+====================
+
+User space can request that KVM provide the paravirtualized time interface to
+a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
+
+ struct kvm_create_device pvtime_device = {
+ .type = KVM_DEV_TYPE_ARM_PV_TIME,
+ .attr = 0,
+ .flags = 0,
+ };
+
+ pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
+
+The guest IPA of the structures must be given to KVM. This is the address of
+the LPT structure and the base address of an array of stolen time structures
+(one for each VCPU). For example:
+
+ struct kvm_device_attr lpt_base = {
+ .group = KVM_DEV_ARM_PV_TIME_PADDR,
+ .attr = KVM_DEV_ARM_PV_TIME_LPT,
+ .addr = (u64)(unsigned long)&lpt_paddr
+ };
+ struct kvm_device_attr st_base = {
+ .group = KVM_DEV_ARM_PV_TIME_PADDR,
+ .attr = KVM_DEV_ARM_PV_TIME_ST,
+ .addr = (u64)(unsigned long)&st_paddr
+ };
+
+ ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_base);
+ ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
+
+The paravirtualized frequency of the guest can also be set. By default this
+will be the counter frequency of the host. However when migrating a guest from
+another host, this must be manually set to ensure that the guest sees the same
+frequency.
+
+ u32 frequency;
+
+ struct kvm_device_attr lpt_freq = {
+ .group = KVM_DEV_ARM_PV_TIME_FREQUENCY,
+ .attr = KVM_DEV_ARM_PV_TIME_LPT,
+ .addr = (u64)(unsigned long)&frequency
+ };
+
+ ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_freq);
+
+For migration (or save/restore) of a guest it is necessary to save the contents
+of the shared pages and later restore them. KVM_DEV_ARM_PV_TIME_STATE_SIZE
+provides the size of this data and KVM_DEV_ARM_PV_TIME_STATE allows the state
+to be read/written. The state for stolen time and LPT are accessed separately.
+It is also necessary for the physical address and frequency to be set
+identically when restoring. The kernel will update the structure on first run
+of the vCPU(s) to contain the new coefficients.
+
+ void *save_state(int fd, u64 attr, u32 *size) {
+ struct kvm_device_attr get_size = {
+ .group = KVM_DEV_ARM_PV_TIME_STATE_SIZE,
+ .attr = attr,
+ .addr = (u64)(unsigned long)size
+ };
+
+ ioctl(fd, KVM_GET_DEVICE_ATTR, get_size);
+
+ void *buffer = malloc(*size);
+
+ struct kvm_device_attr get_state = {
+ .group = KVM_DEV_ARM_PV_TIME_STATE,
+ .attr = attr,
+ .addr = (u64)(unsigned long)size
+ };
+
+ ioctl(fd, KVM_GET_DEVICE_ATTR, buffer);
+ }
+
+ void *lpt_state = save_state(pvtime_fd, KVM_DEV_ARM_PV_TIME_LPT, &lpt_size);
+ void *st_state = save_state(pvtime_fd, KVM_DEV_ARM_PV_TIME_ST, &st_size);
+
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-11-28 14:45 ` [PATCH 01/12] KVM: arm64: Document PV-time interface Steven Price
@ 2018-12-03 13:50 ` Andrew Jones
2018-12-03 14:18 ` Marc Zyngier
2018-12-05 12:32 ` Steven Price
0 siblings, 2 replies; 32+ messages in thread
From: Andrew Jones @ 2018-12-03 13:50 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
> We introduce a paravirtualization interface for KVM/arm64 based on the
> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>
> User space can specify a reserved area of memory for the guest and
> inform KVM to populate the memory with information on stolen time and
> Live Physical Time (LPT) that can be used to derive a stable
> counter/timer for a guest subject to migration between hosts with
> different counter frequencies.
>
> A hypercall interface is provided for the guest to interrogate the
> hypervisor's support for this interface and the location of the shared
> memory structures.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Documentation/virtual/kvm/arm/pvtime.txt | 169 +++++++++++++++++++++++
> 1 file changed, 169 insertions(+)
> create mode 100644 Documentation/virtual/kvm/arm/pvtime.txt
>
> diff --git a/Documentation/virtual/kvm/arm/pvtime.txt b/Documentation/virtual/kvm/arm/pvtime.txt
> new file mode 100644
> index 000000000000..1870b904075b
> --- /dev/null
> +++ b/Documentation/virtual/kvm/arm/pvtime.txt
> @@ -0,0 +1,169 @@
> +Paravirtualized time support for arm64
> +======================================
> +
> +Arm specification DEN0057/A defined a standard for paravirtualised time
> +support for Aarch64 guests:
> +
> +https://developer.arm.com/docs/den0057/a
> +
> +KVM/Arm64 implements this specification by providing some hypervisor service
> +calls to support a paravirtualized guest obtaining a view of the amount of
> +time stolen from its execution and a concept of Live Physical Time (LPT) which
> +represents time during which the guest is running and works across migrations.
> +
> +Three new SMCCC compatible hypercalls are defined:
> +
> +PV_FEATURES 0xC5000020
> +PV_TIME_LPT 0xC5000021
> +PV_TIME_ST 0xC5000022
> +
> +These are only available in the SMC64/HVC64 calling convention as
> +paravirtualized time is not available to 32 bit Arm guests.
> +
> +PV_FEATURES
> + Function ID: (uint32) : 0xC5000020
> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
> + PV-time feature is supported by the hypervisor.
What happens when a new guest tries this hypercall on an old host? Why not
return a bitmap of all supported features so the guest doesn't have to
do multiple hypercalls to build its bitmap?
> +
> +PV_TIME_LPT
> + Function ID: (uint32) : 0xC5000021
> + Flags: (uint32) : Bit[0]: Request migration interrupts
> + (not currently supported by KVM)
> + Return value: (int64) : IPA of the shared live physical time data
> + structure or negative error code on failure:
> + NOT_SUPPORTED (-1)
> + INVALID_PARAMETERS (-2)
> +
> +PV_TIME_ST
> + Function ID: (uint32) : 0xC5000022
> + Return value: (int64) : IPA of the stolen time data structure for this
> + (V)CPU. On failure:
> + NOT_SUPPORTED (-1)
> +
> +Live Physical Time
> +------------------
> +
> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
> +
> + Field | Byte Length | Byte Offset | Description
> + --------------- | ----------- | ----------- | -------------------------------
> + Revision | 4 | 0 | Must be 0 for this revision
> + Attributes | 4 | 4 | Must be 0
> + sequence_number | 8 | 8 | Bit 0: reserved
> + | | | Bits 1:63 number of migrations
> + scale_mult | 8 | 16 | Multiplier to scale from native
> + | | | frequency to PV frequency
> + shift | 4 | 24 | Shift applied before multiplier
> + Reserved | 4 | 28 | Must be 0
> + Fn | 8 | 32 | Native frequency
> + Fpv | 8 | 40 | Paravirtualized frequency seen
> + | | | by guest
> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
> + | | | divide by Fpv
Here's kvmclock's struct
struct pvclock_vcpu_time_info {
u32 version;
u32 pad0;
u64 tsc_timestamp;
u64 system_time;
u32 tsc_to_system_mul;
s8 tsc_shift;
u8 flags;
u8 pad[2];
}
Revision =>
Attributes =>
sequence_number => version
scale_mult => tsc_to_system_mul (this is reversed, but OK)
shift => tsc_shift (also reversed)
Reserved =>
Fn => (pvclock doesn't have, but does have system_time)
Fpv =>
div_by_fpv_mult =>
I haven't thought about this enough yet to be sure kvmclock's fields
are sufficient, but several look close - although the 'tsc' naming
isn't nice. Also, the pvclock struct could be extended by adding an
'extended' flag to 'flags', and then appending more fields.
> +
> +Where scale_mult is defined as 2^(64-shift) * Fpv / Fn
> +
> +The structure will be updated by the hypervisor whenever the guest is migrated
> +to a new host. It will be present within a reserved region of the normal
> +memory given to the guest. The guest should not attempt to write into this
> +memory.
> +
> +Stolen Time
> +-----------
> +
> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
> +
> + Field | Byte Length | Byte Offset | Description
> + ----------- | ----------- | ----------- | --------------------------
> + Revision | 4 | 0 | Must be 0 for version 0.1
> + Attributes | 4 | 4 | Must be 0
> + Stolen time | 8 | 8 | Stolen time in unsigned
> + | | | nanoseconds indicating how
> + | | | much time this VCPU thread
> + | | | was involuntarily not
> + | | | running on a physical CPU.
> +
> +The structure will be updated by the hypervisor periodically as time is stolen
> +from the VCPU. It will be present within a reserved region of the normal
> +memory given to the guest. The guest should not attempt to write into this
> +memory. There is a structure by VCPU of the guest.
> +
> +User space interface
> +====================
> +
> +User space can request that KVM provide the paravirtualized time interface to
> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
> +
> + struct kvm_create_device pvtime_device = {
> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
> + .attr = 0,
> + .flags = 0,
> + };
> +
> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
> +
> +The guest IPA of the structures must be given to KVM. This is the address of
> +the LPT structure and the base address of an array of stolen time structures
> +(one for each VCPU). For example:
> +
> + struct kvm_device_attr lpt_base = {
> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
> + .attr = KVM_DEV_ARM_PV_TIME_LPT,
> + .addr = (u64)(unsigned long)&lpt_paddr
> + };
> + struct kvm_device_attr st_base = {
> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
> + .attr = KVM_DEV_ARM_PV_TIME_ST,
> + .addr = (u64)(unsigned long)&st_paddr
> + };
> +
> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_base);
> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
> +
> +The paravirtualized frequency of the guest can also be set. By default this
> +will be the counter frequency of the host. However when migrating a guest from
> +another host, this must be manually set to ensure that the guest sees the same
> +frequency.
> +
> + u32 frequency;
> +
> + struct kvm_device_attr lpt_freq = {
> + .group = KVM_DEV_ARM_PV_TIME_FREQUENCY,
> + .attr = KVM_DEV_ARM_PV_TIME_LPT,
> + .addr = (u64)(unsigned long)&frequency
> + };
> +
> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_freq);
> +
> +For migration (or save/restore) of a guest it is necessary to save the contents
> +of the shared pages and later restore them. KVM_DEV_ARM_PV_TIME_STATE_SIZE
> +provides the size of this data and KVM_DEV_ARM_PV_TIME_STATE allows the state
> +to be read/written. The state for stolen time and LPT are accessed separately.
I guess there's no harm in userspace accessing these separately, but I
hope we'll be trying to put all the PV structures on one shared page
for the guest.
Thanks,
drew
> +It is also necessary for the physical address and frequency to be set
> +identically when restoring. The kernel will update the structure on first run
> +of the vCPU(s) to contain the new coefficients.
> +
> + void *save_state(int fd, u64 attr, u32 *size) {
> + struct kvm_device_attr get_size = {
> + .group = KVM_DEV_ARM_PV_TIME_STATE_SIZE,
> + .attr = attr,
> + .addr = (u64)(unsigned long)size
> + };
> +
> + ioctl(fd, KVM_GET_DEVICE_ATTR, get_size);
> +
> + void *buffer = malloc(*size);
> +
> + struct kvm_device_attr get_state = {
> + .group = KVM_DEV_ARM_PV_TIME_STATE,
> + .attr = attr,
> + .addr = (u64)(unsigned long)size
> + };
> +
> + ioctl(fd, KVM_GET_DEVICE_ATTR, buffer);
> + }
> +
> + void *lpt_state = save_state(pvtime_fd, KVM_DEV_ARM_PV_TIME_LPT, &lpt_size);
> + void *st_state = save_state(pvtime_fd, KVM_DEV_ARM_PV_TIME_ST, &st_size);
> +
> --
> 2.19.2
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-12-03 13:50 ` Andrew Jones
@ 2018-12-03 14:18 ` Marc Zyngier
2018-12-03 15:16 ` Andrew Jones
2018-12-05 12:32 ` Steven Price
1 sibling, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-12-03 14:18 UTC (permalink / raw)
To: Andrew Jones, Steven Price
Cc: Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel
Hi Drew,
On 03/12/2018 13:50, Andrew Jones wrote:
> On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
>> We introduce a paravirtualization interface for KVM/arm64 based on the
>> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>>
>> User space can specify a reserved area of memory for the guest and
>> inform KVM to populate the memory with information on stolen time and
>> Live Physical Time (LPT) that can be used to derive a stable
>> counter/timer for a guest subject to migration between hosts with
>> different counter frequencies.
>>
>> A hypercall interface is provided for the guest to interrogate the
>> hypervisor's support for this interface and the location of the shared
>> memory structures.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Documentation/virtual/kvm/arm/pvtime.txt | 169 +++++++++++++++++++++++
>> 1 file changed, 169 insertions(+)
>> create mode 100644 Documentation/virtual/kvm/arm/pvtime.txt
>>
>> diff --git a/Documentation/virtual/kvm/arm/pvtime.txt b/Documentation/virtual/kvm/arm/pvtime.txt
>> new file mode 100644
>> index 000000000000..1870b904075b
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/arm/pvtime.txt
>> @@ -0,0 +1,169 @@
>> +Paravirtualized time support for arm64
>> +======================================
>> +
>> +Arm specification DEN0057/A defined a standard for paravirtualised time
>> +support for Aarch64 guests:
>> +
>> +https://developer.arm.com/docs/den0057/a
>> +
>> +KVM/Arm64 implements this specification by providing some hypervisor service
>> +calls to support a paravirtualized guest obtaining a view of the amount of
>> +time stolen from its execution and a concept of Live Physical Time (LPT) which
>> +represents time during which the guest is running and works across migrations.
>> +
>> +Three new SMCCC compatible hypercalls are defined:
>> +
>> +PV_FEATURES 0xC5000020
>> +PV_TIME_LPT 0xC5000021
>> +PV_TIME_ST 0xC5000022
>> +
>> +These are only available in the SMC64/HVC64 calling convention as
>> +paravirtualized time is not available to 32 bit Arm guests.
>> +
>> +PV_FEATURES
>> + Function ID: (uint32) : 0xC5000020
>> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
>> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
>> + PV-time feature is supported by the hypervisor.
>
> What happens when a new guest tries this hypercall on an old host? Why not
> return a bitmap of all supported features so the guest doesn't have to
> do multiple hypercalls to build its bitmap?
A guest doesn't call this directly. It first checks that the service is
implemented, and only if it is, it can call it. It is the exact same
mechanism we use for other hypercalls (such as the Spectre mitigation
services). If the guest bypasses the discovery mechanism, tough.
>
>> +
>> +PV_TIME_LPT
>> + Function ID: (uint32) : 0xC5000021
>> + Flags: (uint32) : Bit[0]: Request migration interrupts
>> + (not currently supported by KVM)
>> + Return value: (int64) : IPA of the shared live physical time data
>> + structure or negative error code on failure:
>> + NOT_SUPPORTED (-1)
>> + INVALID_PARAMETERS (-2)
>> +
>> +PV_TIME_ST
>> + Function ID: (uint32) : 0xC5000022
>> + Return value: (int64) : IPA of the stolen time data structure for this
>> + (V)CPU. On failure:
>> + NOT_SUPPORTED (-1)
>> +
>> +Live Physical Time
>> +------------------
>> +
>> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
>> +
>> + Field | Byte Length | Byte Offset | Description
>> + --------------- | ----------- | ----------- | -------------------------------
>> + Revision | 4 | 0 | Must be 0 for this revision
>> + Attributes | 4 | 4 | Must be 0
>> + sequence_number | 8 | 8 | Bit 0: reserved
>> + | | | Bits 1:63 number of migrations
>> + scale_mult | 8 | 16 | Multiplier to scale from native
>> + | | | frequency to PV frequency
>> + shift | 4 | 24 | Shift applied before multiplier
>> + Reserved | 4 | 28 | Must be 0
>> + Fn | 8 | 32 | Native frequency
>> + Fpv | 8 | 40 | Paravirtualized frequency seen
>> + | | | by guest
>> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
>> + | | | divide by Fpv
>
> Here's kvmclock's struct
>
> struct pvclock_vcpu_time_info {
> u32 version;
> u32 pad0;
> u64 tsc_timestamp;
> u64 system_time;
> u32 tsc_to_system_mul;
> s8 tsc_shift;
> u8 flags;
> u8 pad[2];
> }
>
> Revision =>
> Attributes =>
> sequence_number => version
> scale_mult => tsc_to_system_mul (this is reversed, but OK)
> shift => tsc_shift (also reversed)
> Reserved =>
> Fn => (pvclock doesn't have, but does have system_time)
> Fpv =>
> div_by_fpv_mult =>
>
> I haven't thought about this enough yet to be sure kvmclock's fields
> are sufficient, but several look close - although the 'tsc' naming
> isn't nice. Also, the pvclock struct could be extended by adding an
> 'extended' flag to 'flags', and then appending more fields.
One important thing is that this is not a KVM PV time implementation.
This is something generic for the ARM architecture. Any ARM hypervisor
should be able to implement this.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-12-03 14:18 ` Marc Zyngier
@ 2018-12-03 15:16 ` Andrew Jones
2018-12-03 15:23 ` Marc Zyngier
0 siblings, 1 reply; 32+ messages in thread
From: Andrew Jones @ 2018-12-03 15:16 UTC (permalink / raw)
To: Marc Zyngier
Cc: Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel,
Steven Price
On Mon, Dec 03, 2018 at 02:18:25PM +0000, Marc Zyngier wrote:
> On 03/12/2018 13:50, Andrew Jones wrote:
> > On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
> >> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
> >> +
> >> + Field | Byte Length | Byte Offset | Description
> >> + --------------- | ----------- | ----------- | -------------------------------
> >> + Revision | 4 | 0 | Must be 0 for this revision
> >> + Attributes | 4 | 4 | Must be 0
> >> + sequence_number | 8 | 8 | Bit 0: reserved
> >> + | | | Bits 1:63 number of migrations
> >> + scale_mult | 8 | 16 | Multiplier to scale from native
> >> + | | | frequency to PV frequency
> >> + shift | 4 | 24 | Shift applied before multiplier
> >> + Reserved | 4 | 28 | Must be 0
> >> + Fn | 8 | 32 | Native frequency
> >> + Fpv | 8 | 40 | Paravirtualized frequency seen
> >> + | | | by guest
> >> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
> >> + | | | divide by Fpv
> >
> > Here's kvmclock's struct
> >
> > struct pvclock_vcpu_time_info {
> > u32 version;
> > u32 pad0;
> > u64 tsc_timestamp;
> > u64 system_time;
> > u32 tsc_to_system_mul;
> > s8 tsc_shift;
> > u8 flags;
> > u8 pad[2];
> > }
> >
> > Revision =>
> > Attributes =>
> > sequence_number => version
> > scale_mult => tsc_to_system_mul (this is reversed, but OK)
> > shift => tsc_shift (also reversed)
> > Reserved =>
> > Fn => (pvclock doesn't have, but does have system_time)
> > Fpv =>
> > div_by_fpv_mult =>
> >
> > I haven't thought about this enough yet to be sure kvmclock's fields
> > are sufficient, but several look close - although the 'tsc' naming
> > isn't nice. Also, the pvclock struct could be extended by adding an
> > 'extended' flag to 'flags', and then appending more fields.
>
> One important thing is that this is not a KVM PV time implementation.
> This is something generic for the ARM architecture. Any ARM hypervisor
> should be able to implement this.
>
I agree with that, but xen also uses the same pvclock structure on x86.
Thanks,
drew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-12-03 15:16 ` Andrew Jones
@ 2018-12-03 15:23 ` Marc Zyngier
2018-12-03 15:52 ` Andrew Jones
0 siblings, 1 reply; 32+ messages in thread
From: Marc Zyngier @ 2018-12-03 15:23 UTC (permalink / raw)
To: Andrew Jones
Cc: Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel,
Steven Price
On 03/12/2018 15:16, Andrew Jones wrote:
> On Mon, Dec 03, 2018 at 02:18:25PM +0000, Marc Zyngier wrote:
>> On 03/12/2018 13:50, Andrew Jones wrote:
>>> On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
>>>> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
>>>> +
>>>> + Field | Byte Length | Byte Offset | Description
>>>> + --------------- | ----------- | ----------- | -------------------------------
>>>> + Revision | 4 | 0 | Must be 0 for this revision
>>>> + Attributes | 4 | 4 | Must be 0
>>>> + sequence_number | 8 | 8 | Bit 0: reserved
>>>> + | | | Bits 1:63 number of migrations
>>>> + scale_mult | 8 | 16 | Multiplier to scale from native
>>>> + | | | frequency to PV frequency
>>>> + shift | 4 | 24 | Shift applied before multiplier
>>>> + Reserved | 4 | 28 | Must be 0
>>>> + Fn | 8 | 32 | Native frequency
>>>> + Fpv | 8 | 40 | Paravirtualized frequency seen
>>>> + | | | by guest
>>>> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
>>>> + | | | divide by Fpv
>>>
>>> Here's kvmclock's struct
>>>
>>> struct pvclock_vcpu_time_info {
>>> u32 version;
>>> u32 pad0;
>>> u64 tsc_timestamp;
>>> u64 system_time;
>>> u32 tsc_to_system_mul;
>>> s8 tsc_shift;
>>> u8 flags;
>>> u8 pad[2];
>>> }
>>>
>>> Revision =>
>>> Attributes =>
>>> sequence_number => version
>>> scale_mult => tsc_to_system_mul (this is reversed, but OK)
>>> shift => tsc_shift (also reversed)
>>> Reserved =>
>>> Fn => (pvclock doesn't have, but does have system_time)
>>> Fpv =>
>>> div_by_fpv_mult =>
>>>
>>> I haven't thought about this enough yet to be sure kvmclock's fields
>>> are sufficient, but several look close - although the 'tsc' naming
>>> isn't nice. Also, the pvclock struct could be extended by adding an
>>> 'extended' flag to 'flags', and then appending more fields.
>>
>> One important thing is that this is not a KVM PV time implementation.
>> This is something generic for the ARM architecture. Any ARM hypervisor
>> should be able to implement this.
>>
>
> I agree with that, but xen also uses the same pvclock structure on x86.
Which makes sense.
Each architecture has a way to express its PV time based on its own
requirements, and I'd expect Xen on arm64 to use the ARM data structure
(/me eyes the Xen/ARM maintainer...). I thought that what you were
advocating was to use the x86 layout on ARM, which didn't make complete
sense to me.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-12-03 15:23 ` Marc Zyngier
@ 2018-12-03 15:52 ` Andrew Jones
0 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2018-12-03 15:52 UTC (permalink / raw)
To: Marc Zyngier
Cc: Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel,
Steven Price
On Mon, Dec 03, 2018 at 03:23:41PM +0000, Marc Zyngier wrote:
> On 03/12/2018 15:16, Andrew Jones wrote:
> > On Mon, Dec 03, 2018 at 02:18:25PM +0000, Marc Zyngier wrote:
> >> On 03/12/2018 13:50, Andrew Jones wrote:
> >>> On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
> >>>> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
> >>>> +
> >>>> + Field | Byte Length | Byte Offset | Description
> >>>> + --------------- | ----------- | ----------- | -------------------------------
> >>>> + Revision | 4 | 0 | Must be 0 for this revision
> >>>> + Attributes | 4 | 4 | Must be 0
> >>>> + sequence_number | 8 | 8 | Bit 0: reserved
> >>>> + | | | Bits 1:63 number of migrations
> >>>> + scale_mult | 8 | 16 | Multiplier to scale from native
> >>>> + | | | frequency to PV frequency
> >>>> + shift | 4 | 24 | Shift applied before multiplier
> >>>> + Reserved | 4 | 28 | Must be 0
> >>>> + Fn | 8 | 32 | Native frequency
> >>>> + Fpv | 8 | 40 | Paravirtualized frequency seen
> >>>> + | | | by guest
> >>>> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
> >>>> + | | | divide by Fpv
> >>>
> >>> Here's kvmclock's struct
> >>>
> >>> struct pvclock_vcpu_time_info {
> >>> u32 version;
> >>> u32 pad0;
> >>> u64 tsc_timestamp;
> >>> u64 system_time;
> >>> u32 tsc_to_system_mul;
> >>> s8 tsc_shift;
> >>> u8 flags;
> >>> u8 pad[2];
> >>> }
> >>>
> >>> Revision =>
> >>> Attributes =>
> >>> sequence_number => version
> >>> scale_mult => tsc_to_system_mul (this is reversed, but OK)
> >>> shift => tsc_shift (also reversed)
> >>> Reserved =>
> >>> Fn => (pvclock doesn't have, but does have system_time)
> >>> Fpv =>
> >>> div_by_fpv_mult =>
> >>>
> >>> I haven't thought about this enough yet to be sure kvmclock's fields
> >>> are sufficient, but several look close - although the 'tsc' naming
> >>> isn't nice. Also, the pvclock struct could be extended by adding an
> >>> 'extended' flag to 'flags', and then appending more fields.
> >>
> >> One important thing is that this is not a KVM PV time implementation.
> >> This is something generic for the ARM architecture. Any ARM hypervisor
> >> should be able to implement this.
> >>
> >
> > I agree with that, but xen also uses the same pvclock structure on x86.
>
> Which makes sense.
>
> Each architecture has a way to express its PV time based on its own
> requirements, and I'd expect Xen on arm64 to use the ARM data structure
> (/me eyes the Xen/ARM maintainer...). I thought that what you were
> advocating was to use the x86 layout on ARM, which didn't make complete
> sense to me.
I sort of was. I was wondering if pvclock shouldn't be improved in a
backward compatible way by using the flags field to extend it. Field names
could change and new fields can be appended. Then, when x86 kvm and xen
are updated to use the extended pvclock struct, there may be more
opportunity for code sharing across architectures - perhaps much of
kvmclock could be shared. Of course I'm just thinking out loud, and doing
more of the out loud part than the thinking...
Hmm, I do see that arm-xen has already adopted the x86 layouts without
modification and also has a TODO saying the struct definitions should put
somewhere arch neutral.
Thanks,
drew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 01/12] KVM: arm64: Document PV-time interface
2018-12-03 13:50 ` Andrew Jones
2018-12-03 14:18 ` Marc Zyngier
@ 2018-12-05 12:32 ` Steven Price
1 sibling, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-12-05 12:32 UTC (permalink / raw)
To: Andrew Jones
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On 03/12/2018 13:50, Andrew Jones wrote:
> On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
>> We introduce a paravirtualization interface for KVM/arm64 based on the
>> "Arm Paravirtualized Time for Arm-Base Systems" specification DEN 0057A.
>>
>> User space can specify a reserved area of memory for the guest and
>> inform KVM to populate the memory with information on stolen time and
>> Live Physical Time (LPT) that can be used to derive a stable
>> counter/timer for a guest subject to migration between hosts with
>> different counter frequencies.
>>
>> A hypercall interface is provided for the guest to interrogate the
>> hypervisor's support for this interface and the location of the shared
>> memory structures.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Documentation/virtual/kvm/arm/pvtime.txt | 169 +++++++++++++++++++++++
>> 1 file changed, 169 insertions(+)
>> create mode 100644 Documentation/virtual/kvm/arm/pvtime.txt
>>
>> diff --git a/Documentation/virtual/kvm/arm/pvtime.txt b/Documentation/virtual/kvm/arm/pvtime.txt
>> new file mode 100644
>> index 000000000000..1870b904075b
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/arm/pvtime.txt
>> @@ -0,0 +1,169 @@
>> +Paravirtualized time support for arm64
>> +======================================
>> +
>> +Arm specification DEN0057/A defined a standard for paravirtualised time
>> +support for Aarch64 guests:
>> +
>> +https://developer.arm.com/docs/den0057/a
>> +
>> +KVM/Arm64 implements this specification by providing some hypervisor service
>> +calls to support a paravirtualized guest obtaining a view of the amount of
>> +time stolen from its execution and a concept of Live Physical Time (LPT) which
>> +represents time during which the guest is running and works across migrations.
>> +
>> +Three new SMCCC compatible hypercalls are defined:
>> +
>> +PV_FEATURES 0xC5000020
>> +PV_TIME_LPT 0xC5000021
>> +PV_TIME_ST 0xC5000022
>> +
>> +These are only available in the SMC64/HVC64 calling convention as
>> +paravirtualized time is not available to 32 bit Arm guests.
>> +
>> +PV_FEATURES
>> + Function ID: (uint32) : 0xC5000020
>> + PV_func_id: (uint32) : Either PV_TIME_LPT or PV_TIME_ST
>> + Return value: (int32) : NOT_SUPPORTED (-1) or SUCCESS (0) if the relevant
>> + PV-time feature is supported by the hypervisor.
>
> What happens when a new guest tries this hypercall on an old host? Why not
> return a bitmap of all supported features so the guest doesn't have to
> do multiple hypercalls to build its bitmap?
As Marc pointed out - a guest shouldn't (the discover mechanism requires
using SMCCC ARCH_FEATURES). However if a guest was to ignore this it
would still receive the default NOT_SUPPORTED (-1) response.
The design of PV_FEATURES matches the existing SMCCC ARCH_FEATURES so
does require multiple calls. It would be possible to return a bitmap but
that would limit the room for expansion. It's not exactly a performance
critical path ;)
>> +
>> +PV_TIME_LPT
>> + Function ID: (uint32) : 0xC5000021
>> + Flags: (uint32) : Bit[0]: Request migration interrupts
>> + (not currently supported by KVM)
>> + Return value: (int64) : IPA of the shared live physical time data
>> + structure or negative error code on failure:
>> + NOT_SUPPORTED (-1)
>> + INVALID_PARAMETERS (-2)
>> +
>> +PV_TIME_ST
>> + Function ID: (uint32) : 0xC5000022
>> + Return value: (int64) : IPA of the stolen time data structure for this
>> + (V)CPU. On failure:
>> + NOT_SUPPORTED (-1)
>> +
>> +Live Physical Time
>> +------------------
>> +
>> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
>> +
>> + Field | Byte Length | Byte Offset | Description
>> + --------------- | ----------- | ----------- | -------------------------------
>> + Revision | 4 | 0 | Must be 0 for this revision
>> + Attributes | 4 | 4 | Must be 0
>> + sequence_number | 8 | 8 | Bit 0: reserved
>> + | | | Bits 1:63 number of migrations
>> + scale_mult | 8 | 16 | Multiplier to scale from native
>> + | | | frequency to PV frequency
>> + shift | 4 | 24 | Shift applied before multiplier
>> + Reserved | 4 | 28 | Must be 0
>> + Fn | 8 | 32 | Native frequency
>> + Fpv | 8 | 40 | Paravirtualized frequency seen
>> + | | | by guest
>> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
>> + | | | divide by Fpv
>
> Here's kvmclock's struct
>
> struct pvclock_vcpu_time_info {
> u32 version;
> u32 pad0;
> u64 tsc_timestamp;
> u64 system_time;
> u32 tsc_to_system_mul;
> s8 tsc_shift;
> u8 flags;
> u8 pad[2];
> }
>
> Revision =>
> Attributes =>
> sequence_number => version
> scale_mult => tsc_to_system_mul (this is reversed, but OK)
> shift => tsc_shift (also reversed)
> Reserved =>
> Fn => (pvclock doesn't have, but does have system_time)
> Fpv =>
> div_by_fpv_mult =>
>
> I haven't thought about this enough yet to be sure kvmclock's fields
> are sufficient, but several look close - although the 'tsc' naming
> isn't nice. Also, the pvclock struct could be extended by adding an
> 'extended' flag to 'flags', and then appending more fields.
While it may be possible to try to map the fields onto the existing
structure it's not going to be a great fit, and I don't really see the
point. Unless the meaning is going to exactly match x86 then it would be
the same structure in name only.
>> +
>> +Where scale_mult is defined as 2^(64-shift) * Fpv / Fn
>> +
>> +The structure will be updated by the hypervisor whenever the guest is migrated
>> +to a new host. It will be present within a reserved region of the normal
>> +memory given to the guest. The guest should not attempt to write into this
>> +memory.
>> +
>> +Stolen Time
>> +-----------
>> +
>> +The structure pointed to by the PV_TIME_ST hypercall is as follows:
>> +
>> + Field | Byte Length | Byte Offset | Description
>> + ----------- | ----------- | ----------- | --------------------------
>> + Revision | 4 | 0 | Must be 0 for version 0.1
>> + Attributes | 4 | 4 | Must be 0
>> + Stolen time | 8 | 8 | Stolen time in unsigned
>> + | | | nanoseconds indicating how
>> + | | | much time this VCPU thread
>> + | | | was involuntarily not
>> + | | | running on a physical CPU.
>> +
>> +The structure will be updated by the hypervisor periodically as time is stolen
>> +from the VCPU. It will be present within a reserved region of the normal
>> +memory given to the guest. The guest should not attempt to write into this
>> +memory. There is a structure by VCPU of the guest.
>> +
>> +User space interface
>> +====================
>> +
>> +User space can request that KVM provide the paravirtualized time interface to
>> +a guest by creating a KVM_DEV_TYPE_ARM_PV_TIME device, for example:
>> +
>> + struct kvm_create_device pvtime_device = {
>> + .type = KVM_DEV_TYPE_ARM_PV_TIME,
>> + .attr = 0,
>> + .flags = 0,
>> + };
>> +
>> + pvtime_fd = ioctl(vm_fd, KVM_CREATE_DEVICE, &pvtime_device);
>> +
>> +The guest IPA of the structures must be given to KVM. This is the address of
>> +the LPT structure and the base address of an array of stolen time structures
>> +(one for each VCPU). For example:
>> +
>> + struct kvm_device_attr lpt_base = {
>> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
>> + .attr = KVM_DEV_ARM_PV_TIME_LPT,
>> + .addr = (u64)(unsigned long)&lpt_paddr
>> + };
>> + struct kvm_device_attr st_base = {
>> + .group = KVM_DEV_ARM_PV_TIME_PADDR,
>> + .attr = KVM_DEV_ARM_PV_TIME_ST,
>> + .addr = (u64)(unsigned long)&st_paddr
>> + };
>> +
>> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_base);
>> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &st_base);
>> +
>> +The paravirtualized frequency of the guest can also be set. By default this
>> +will be the counter frequency of the host. However when migrating a guest from
>> +another host, this must be manually set to ensure that the guest sees the same
>> +frequency.
>> +
>> + u32 frequency;
>> +
>> + struct kvm_device_attr lpt_freq = {
>> + .group = KVM_DEV_ARM_PV_TIME_FREQUENCY,
>> + .attr = KVM_DEV_ARM_PV_TIME_LPT,
>> + .addr = (u64)(unsigned long)&frequency
>> + };
>> +
>> + ioctl(pvtime_fd, KVM_SET_DEVICE_ATTR, &lpt_freq);
>> +
>> +For migration (or save/restore) of a guest it is necessary to save the contents
>> +of the shared pages and later restore them. KVM_DEV_ARM_PV_TIME_STATE_SIZE
>> +provides the size of this data and KVM_DEV_ARM_PV_TIME_STATE allows the state
>> +to be read/written. The state for stolen time and LPT are accessed separately.
>
> I guess there's no harm in userspace accessing these separately, but I
> hope we'll be trying to put all the PV structures on one shared page
> for the guest.
Note that it's possible to only just one of the PV mechanisms. In
particular if the hardware in the future supports clock scaling then we
won't need LPT any more so you would just want to expose stolen time.
The API allows the hypervisor to place the structure(s) arbitrarily in
physical memory - the requirement is just to present them at the correct
IPA to the guest. The implementation in this series uses one page for
LPT and (at least) one page for stolen time.
Thanks for the review,
Steve
> Thanks,
> drew
>
>> +It is also necessary for the physical address and frequency to be set
>> +identically when restoring. The kernel will update the structure on first run
>> +of the vCPU(s) to contain the new coefficients.
>> +
>> + void *save_state(int fd, u64 attr, u32 *size) {
>> + struct kvm_device_attr get_size = {
>> + .group = KVM_DEV_ARM_PV_TIME_STATE_SIZE,
>> + .attr = attr,
>> + .addr = (u64)(unsigned long)size
>> + };
>> +
>> + ioctl(fd, KVM_GET_DEVICE_ATTR, get_size);
>> +
>> + void *buffer = malloc(*size);
>> +
>> + struct kvm_device_attr get_state = {
>> + .group = KVM_DEV_ARM_PV_TIME_STATE,
>> + .attr = attr,
>> + .addr = (u64)(unsigned long)size
>> + };
>> +
>> + ioctl(fd, KVM_GET_DEVICE_ATTR, buffer);
>> + }
>> +
>> + void *lpt_state = save_state(pvtime_fd, KVM_DEV_ARM_PV_TIME_LPT, &lpt_size);
>> + void *st_state = save_state(pvtime_fd, KVM_DEV_ARM_PV_TIME_ST, &st_size);
>> +
>> --
>> 2.19.2
>>
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm@lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 02/12] KVM: arm/arm64: Factor out hypercall handling from PSCI code
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
2018-11-28 14:45 ` [PATCH 01/12] KVM: arm64: Document PV-time interface Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-12-03 16:02 ` Andrew Jones
2018-11-28 14:45 ` [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls Steven Price
` (11 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@arm.com>
We currently intertwine the KVM PSCI implementation with the general
dispatch of hypercall handling, which makes perfect sense because PSCI
is the only category of hypercalls we support.
However, as we are about to support additional hypercalls, factor out
this functionality into a separate hypercall handler file.
Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
[steven.price at arm.com: rebased]
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm/kvm/Makefile | 2 +-
arch/arm/kvm/handle_exit.c | 2 +-
arch/arm64/kvm/Makefile | 1 +
arch/arm64/kvm/handle_exit.c | 4 +-
include/kvm/arm_hypercalls.h | 43 ++++++++++++++++++++
include/kvm/arm_psci.h | 2 +-
virt/kvm/arm/hypercalls.c | 51 ++++++++++++++++++++++++
virt/kvm/arm/psci.c | 76 +-----------------------------------
8 files changed, 102 insertions(+), 79 deletions(-)
create mode 100644 include/kvm/arm_hypercalls.h
create mode 100644 virt/kvm/arm/hypercalls.c
diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index 48de846f2246..5994f3b3d375 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -24,7 +24,7 @@ obj-y += kvm-arm.o init.o interrupts.o
obj-y += handle_exit.o guest.o emulate.o reset.o
obj-y += coproc.o coproc_a15.o coproc_a7.o vgic-v3-coproc.o
obj-y += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.o
-obj-y += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
+obj-y += $(KVM)/arm/psci.o $(KVM)/arm/perf.o $(KVM)/arm/hypercalls.o
obj-y += $(KVM)/arm/aarch32.o
obj-y += $(KVM)/arm/vgic/vgic.o
diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 910bd8dabb3c..404c67a06a2f 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -21,7 +21,7 @@
#include <asm/kvm_emulate.h>
#include <asm/kvm_coproc.h>
#include <asm/kvm_mmu.h>
-#include <kvm/arm_psci.h>
+#include <kvm/arm_hypercalls.h>
#include <trace/events/kvm.h>
#include "trace.h"
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 0f2a135ba15b..45bb68f6c161 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_KVM_ARM_HOST) += hyp/
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hypercalls.o
kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 35a81bebd02b..5f5e426cab3b 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -22,8 +22,6 @@
#include <linux/kvm.h>
#include <linux/kvm_host.h>
-#include <kvm/arm_psci.h>
-
#include <asm/esr.h>
#include <asm/exception.h>
#include <asm/kvm_asm.h>
@@ -33,6 +31,8 @@
#include <asm/debug-monitors.h>
#include <asm/traps.h>
+#include <kvm/arm_hypercalls.h>
+
#define CREATE_TRACE_POINTS
#include "trace.h"
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
new file mode 100644
index 000000000000..e5f7f81196b6
--- /dev/null
+++ b/include/kvm/arm_hypercalls.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Arm Ltd. */
+
+#ifndef __KVM_ARM_HYPERCALLS_H
+#define __KVM_ARM_HYPERCALLS_H
+
+#include <asm/kvm_emulate.h>
+
+int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+
+static inline u32 smccc_get_function(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 0);
+}
+
+static inline unsigned long smccc_get_arg1(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 1);
+}
+
+static inline unsigned long smccc_get_arg2(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 2);
+}
+
+static inline unsigned long smccc_get_arg3(struct kvm_vcpu *vcpu)
+{
+ return vcpu_get_reg(vcpu, 3);
+}
+
+static inline void smccc_set_retval(struct kvm_vcpu *vcpu,
+ unsigned long a0,
+ unsigned long a1,
+ unsigned long a2,
+ unsigned long a3)
+{
+ vcpu_set_reg(vcpu, 0, a0);
+ vcpu_set_reg(vcpu, 1, a1);
+ vcpu_set_reg(vcpu, 2, a2);
+ vcpu_set_reg(vcpu, 3, a3);
+}
+
+#endif
diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index 4b1548129fa2..ac8cd19d2456 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -51,7 +51,7 @@ static inline int kvm_psci_version(struct kvm_vcpu *vcpu, struct kvm *kvm)
}
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
struct kvm_one_reg;
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
new file mode 100644
index 000000000000..153aa7642100
--- /dev/null
+++ b/virt/kvm/arm/hypercalls.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Arm Ltd.
+
+#include <linux/arm-smccc.h>
+#include <linux/kvm_host.h>
+
+#include <asm/kvm_emulate.h>
+
+#include <kvm/arm_hypercalls.h>
+#include <kvm/arm_psci.h>
+
+int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
+{
+ u32 func_id = smccc_get_function(vcpu);
+ u32 val = SMCCC_RET_NOT_SUPPORTED;
+ u32 feature;
+
+ switch (func_id) {
+ case ARM_SMCCC_VERSION_FUNC_ID:
+ val = ARM_SMCCC_VERSION_1_1;
+ break;
+ case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
+ feature = smccc_get_arg1(vcpu);
+ switch (feature) {
+ case ARM_SMCCC_ARCH_WORKAROUND_1:
+ if (kvm_arm_harden_branch_predictor())
+ val = SMCCC_RET_SUCCESS;
+ break;
+ case ARM_SMCCC_ARCH_WORKAROUND_2:
+ switch (kvm_arm_have_ssbd()) {
+ case KVM_SSBD_FORCE_DISABLE:
+ case KVM_SSBD_UNKNOWN:
+ break;
+ case KVM_SSBD_KERNEL:
+ val = SMCCC_RET_SUCCESS;
+ break;
+ case KVM_SSBD_FORCE_ENABLE:
+ case KVM_SSBD_MITIGATED:
+ val = SMCCC_RET_NOT_REQUIRED;
+ break;
+ }
+ break;
+ }
+ break;
+ default:
+ return kvm_psci_call(vcpu);
+ }
+
+ smccc_set_retval(vcpu, val, 0, 0, 0);
+ return 1;
+}
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 9b73d3ad918a..4f24836cb8cd 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -26,6 +26,7 @@
#include <asm/kvm_host.h>
#include <kvm/arm_psci.h>
+#include <kvm/arm_hypercalls.h>
/*
* This is an implementation of the Power State Coordination Interface
@@ -34,38 +35,6 @@
#define AFFINITY_MASK(level) ~((0x1UL << ((level) * MPIDR_LEVEL_BITS)) - 1)
-static u32 smccc_get_function(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 0);
-}
-
-static unsigned long smccc_get_arg1(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 1);
-}
-
-static unsigned long smccc_get_arg2(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 2);
-}
-
-static unsigned long smccc_get_arg3(struct kvm_vcpu *vcpu)
-{
- return vcpu_get_reg(vcpu, 3);
-}
-
-static void smccc_set_retval(struct kvm_vcpu *vcpu,
- unsigned long a0,
- unsigned long a1,
- unsigned long a2,
- unsigned long a3)
-{
- vcpu_set_reg(vcpu, 0, a0);
- vcpu_set_reg(vcpu, 1, a1);
- vcpu_set_reg(vcpu, 2, a2);
- vcpu_set_reg(vcpu, 3, a3);
-}
-
static unsigned long psci_affinity_mask(unsigned long affinity_level)
{
if (affinity_level <= 3)
@@ -388,7 +357,7 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
* Errors:
* -EINVAL: Unrecognized PSCI function
*/
-static int kvm_psci_call(struct kvm_vcpu *vcpu)
+int kvm_psci_call(struct kvm_vcpu *vcpu)
{
switch (kvm_psci_version(vcpu, vcpu->kvm)) {
case KVM_ARM_PSCI_1_0:
@@ -402,47 +371,6 @@ static int kvm_psci_call(struct kvm_vcpu *vcpu)
};
}
-int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
-{
- u32 func_id = smccc_get_function(vcpu);
- u32 val = SMCCC_RET_NOT_SUPPORTED;
- u32 feature;
-
- switch (func_id) {
- case ARM_SMCCC_VERSION_FUNC_ID:
- val = ARM_SMCCC_VERSION_1_1;
- break;
- case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
- feature = smccc_get_arg1(vcpu);
- switch(feature) {
- case ARM_SMCCC_ARCH_WORKAROUND_1:
- if (kvm_arm_harden_branch_predictor())
- val = SMCCC_RET_SUCCESS;
- break;
- case ARM_SMCCC_ARCH_WORKAROUND_2:
- switch (kvm_arm_have_ssbd()) {
- case KVM_SSBD_FORCE_DISABLE:
- case KVM_SSBD_UNKNOWN:
- break;
- case KVM_SSBD_KERNEL:
- val = SMCCC_RET_SUCCESS;
- break;
- case KVM_SSBD_FORCE_ENABLE:
- case KVM_SSBD_MITIGATED:
- val = SMCCC_RET_NOT_REQUIRED;
- break;
- }
- break;
- }
- break;
- default:
- return kvm_psci_call(vcpu);
- }
-
- smccc_set_retval(vcpu, val, 0, 0, 0);
- return 1;
-}
-
int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
{
return 1; /* PSCI version */
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 02/12] KVM: arm/arm64: Factor out hypercall handling from PSCI code
2018-11-28 14:45 ` [PATCH 02/12] KVM: arm/arm64: Factor out hypercall handling from PSCI code Steven Price
@ 2018-12-03 16:02 ` Andrew Jones
0 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2018-12-03 16:02 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:17PM +0000, Steven Price wrote:
> From: Christoffer Dall <christoffer.dall@arm.com>
>
> We currently intertwine the KVM PSCI implementation with the general
> dispatch of hypercall handling, which makes perfect sense because PSCI
> is the only category of hypercalls we support.
>
> However, as we are about to support additional hypercalls, factor out
> this functionality into a separate hypercall handler file.
>
> Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
> [steven.price@arm.com: rebased]
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm/kvm/Makefile | 2 +-
> arch/arm/kvm/handle_exit.c | 2 +-
> arch/arm64/kvm/Makefile | 1 +
> arch/arm64/kvm/handle_exit.c | 4 +-
> include/kvm/arm_hypercalls.h | 43 ++++++++++++++++++++
> include/kvm/arm_psci.h | 2 +-
> virt/kvm/arm/hypercalls.c | 51 ++++++++++++++++++++++++
> virt/kvm/arm/psci.c | 76 +-----------------------------------
> 8 files changed, 102 insertions(+), 79 deletions(-)
> create mode 100644 include/kvm/arm_hypercalls.h
> create mode 100644 virt/kvm/arm/hypercalls.c
>
Reviewed-by: Andrew Jones <drjones@redhat.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
2018-11-28 14:45 ` [PATCH 01/12] KVM: arm64: Document PV-time interface Steven Price
2018-11-28 14:45 ` [PATCH 02/12] KVM: arm/arm64: Factor out hypercall handling from PSCI code Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-12-10 10:27 ` Mark Rutland
2018-11-28 14:45 ` [PATCH 04/12] arm/arm64: Make use of the SMCCC 1.1 wrapper Steven Price
` (10 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
SMCCC 1.1 calls may use either HVC or SMC depending on the PSCI
conduit. Rather than coding this in every call site provide a macro
which uses the correct instruction. The macro also handles the case
where no PSCI conduit is configured returning a not supported error
in res, along with returning the conduit used for the call.
This allow us to remove some duplicated code and will be useful later
when adding paravirtualized time hypervisor calls.
Signed-off-by: Steven Price <steven.price@arm.com>
---
include/linux/arm-smccc.h | 44 +++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 18863d56273c..b047009e7a0a 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -311,5 +311,49 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
#define SMCCC_RET_NOT_SUPPORTED -1
#define SMCCC_RET_NOT_REQUIRED -2
+/* Like arm_smccc_1_1* but always returns SMCCC_RET_NOT_SUPPORTED.
+ * Used when the PSCI conduit is not defined. The empty asm statement
+ * avoids compiler warnings about unused variables.
+ */
+#define __fail_smccc_1_1(...) \
+ do { \
+ __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
+ asm ("" __constraints(__count_args(__VA_ARGS__))); \
+ if (___res) \
+ ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
+ } while (0)
+
+/*
+ * arm_smccc_1_1() - make an SMCCC v1.1 compliant call
+ *
+ * This is a variadic macro taking one to eight source arguments, and
+ * an optional return structure.
+ *
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 3
+ *
+ * This macro will make either an HVC call or an SMC call depending on the
+ * current PSCI conduit. If no valid conduit is available then -1
+ * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
+ *
+ * The return value also provides the conduit that was used.
+ */
+#define arm_smccc_1_1(...) ({ \
+ int method = psci_ops.conduit; \
+ switch (method) { \
+ case PSCI_CONDUIT_HVC: \
+ arm_smccc_1_1_hvc(__VA_ARGS__); \
+ break; \
+ case PSCI_CONDUIT_SMC: \
+ arm_smccc_1_1_smc(__VA_ARGS__); \
+ break; \
+ default: \
+ __fail_smccc_1_1(__VA_ARGS__); \
+ method = PSCI_CONDUIT_NONE; \
+ break; \
+ } \
+ method; \
+ })
+
#endif /*__ASSEMBLY__*/
#endif /*__LINUX_ARM_SMCCC_H*/
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls
2018-11-28 14:45 ` [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls Steven Price
@ 2018-12-10 10:27 ` Mark Rutland
2018-12-10 13:52 ` Steven Price
0 siblings, 1 reply; 32+ messages in thread
From: Mark Rutland @ 2018-12-10 10:27 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:18PM +0000, Steven Price wrote:
> SMCCC 1.1 calls may use either HVC or SMC depending on the PSCI
> conduit. Rather than coding this in every call site provide a macro
> which uses the correct instruction. The macro also handles the case
> where no PSCI conduit is configured returning a not supported error
> in res, along with returning the conduit used for the call.
>
> This allow us to remove some duplicated code and will be useful later
> when adding paravirtualized time hypervisor calls.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> include/linux/arm-smccc.h | 44 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 18863d56273c..b047009e7a0a 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -311,5 +311,49 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> #define SMCCC_RET_NOT_SUPPORTED -1
> #define SMCCC_RET_NOT_REQUIRED -2
>
> +/* Like arm_smccc_1_1* but always returns SMCCC_RET_NOT_SUPPORTED.
> + * Used when the PSCI conduit is not defined. The empty asm statement
> + * avoids compiler warnings about unused variables.
> + */
> +#define __fail_smccc_1_1(...) \
> + do { \
> + __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
> + asm ("" __constraints(__count_args(__VA_ARGS__))); \
> + if (___res) \
> + ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
> + } while (0)
> +
> +/*
> + * arm_smccc_1_1() - make an SMCCC v1.1 compliant call
> + *
> + * This is a variadic macro taking one to eight source arguments, and
> + * an optional return structure.
> + *
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 3
> + *
> + * This macro will make either an HVC call or an SMC call depending on the
> + * current PSCI conduit. If no valid conduit is available then -1
> + * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
> + *
> + * The return value also provides the conduit that was used.
> + */
> +#define arm_smccc_1_1(...) ({ \
As a minor nit, could we please give call this something like
arm_smccc_1_1_call() or arm_smccc_1_1_invoke(), to make it clear what
action this performs?
I had some patches [1] cleaning up the SMCCC API, it would be nice if we
could pick up some of those as preparatory bits, before we spread some
of the current design warts (e.g. SMCCC depending on PSCI definitions).
Thanks,
Mark.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/smccc-cleanup
> + int method = psci_ops.conduit; \
> + switch (method) { \
> + case PSCI_CONDUIT_HVC: \
> + arm_smccc_1_1_hvc(__VA_ARGS__); \
> + break; \
> + case PSCI_CONDUIT_SMC: \
> + arm_smccc_1_1_smc(__VA_ARGS__); \
> + break; \
> + default: \
> + __fail_smccc_1_1(__VA_ARGS__); \
> + method = PSCI_CONDUIT_NONE; \
> + break; \
> + } \
> + method; \
> + })
> +
> #endif /*__ASSEMBLY__*/
> #endif /*__LINUX_ARM_SMCCC_H*/
> --
> 2.19.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls
2018-12-10 10:27 ` Mark Rutland
@ 2018-12-10 13:52 ` Steven Price
0 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-12-10 13:52 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On 10/12/2018 10:27, Mark Rutland wrote:
> On Wed, Nov 28, 2018 at 02:45:18PM +0000, Steven Price wrote:
>> SMCCC 1.1 calls may use either HVC or SMC depending on the PSCI
>> conduit. Rather than coding this in every call site provide a macro
>> which uses the correct instruction. The macro also handles the case
>> where no PSCI conduit is configured returning a not supported error
>> in res, along with returning the conduit used for the call.
>>
>> This allow us to remove some duplicated code and will be useful later
>> when adding paravirtualized time hypervisor calls.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> include/linux/arm-smccc.h | 44 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
>> index 18863d56273c..b047009e7a0a 100644
>> --- a/include/linux/arm-smccc.h
>> +++ b/include/linux/arm-smccc.h
>> @@ -311,5 +311,49 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>> #define SMCCC_RET_NOT_SUPPORTED -1
>> #define SMCCC_RET_NOT_REQUIRED -2
>>
>> +/* Like arm_smccc_1_1* but always returns SMCCC_RET_NOT_SUPPORTED.
>> + * Used when the PSCI conduit is not defined. The empty asm statement
>> + * avoids compiler warnings about unused variables.
>> + */
>> +#define __fail_smccc_1_1(...) \
>> + do { \
>> + __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
>> + asm ("" __constraints(__count_args(__VA_ARGS__))); \
>> + if (___res) \
>> + ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
>> + } while (0)
>> +
>> +/*
>> + * arm_smccc_1_1() - make an SMCCC v1.1 compliant call
>> + *
>> + * This is a variadic macro taking one to eight source arguments, and
>> + * an optional return structure.
>> + *
>> + * @a0-a7: arguments passed in registers 0 to 7
>> + * @res: result values from registers 0 to 3
>> + *
>> + * This macro will make either an HVC call or an SMC call depending on the
>> + * current PSCI conduit. If no valid conduit is available then -1
>> + * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
>> + *
>> + * The return value also provides the conduit that was used.
>> + */
>> +#define arm_smccc_1_1(...) ({ \
>
> As a minor nit, could we please give call this something like
> arm_smccc_1_1_call() or arm_smccc_1_1_invoke(), to make it clear what
> action this performs?
Sure, arm_smccc_1_1_call() works for me.
> I had some patches [1] cleaning up the SMCCC API, it would be nice if we
> could pick up some of those as preparatory bits, before we spread some
> of the current design warts (e.g. SMCCC depending on PSCI definitions).
Looks like a similar approach, just extended to include
s/PSCI_CONDUIT/SMCCC_CONDUIT/ and making arm_smccc_1_1_* return res.
Would you like me to drop this (and the next) patch in favour of yours?
Thanks,
Steve
> Thanks,
> Mark.
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/smccc-cleanup
>
>> + int method = psci_ops.conduit; \
>> + switch (method) { \
>> + case PSCI_CONDUIT_HVC: \
>> + arm_smccc_1_1_hvc(__VA_ARGS__); \
>> + break; \
>> + case PSCI_CONDUIT_SMC: \
>> + arm_smccc_1_1_smc(__VA_ARGS__); \
>> + break; \
>> + default: \
>> + __fail_smccc_1_1(__VA_ARGS__); \
>> + method = PSCI_CONDUIT_NONE; \
>> + break; \
>> + } \
>> + method; \
>> + })
>> +
>> #endif /*__ASSEMBLY__*/
>> #endif /*__LINUX_ARM_SMCCC_H*/
>> --
>> 2.19.2
>>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 04/12] arm/arm64: Make use of the SMCCC 1.1 wrapper
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (2 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-11-28 14:45 ` [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call Steven Price
` (9 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Rather than directly choosing which function to use based on
psci_ops.conduit, use the new arm_smccc_1_1 wrapper instead.
In some cases we still need to do some operations based on the
conduit, but the code duplication is removed.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm/mm/proc-v7-bugs.c | 46 +++++++++++++++------------------
arch/arm64/kernel/cpu_errata.c | 47 +++++++++-------------------------
2 files changed, 32 insertions(+), 61 deletions(-)
diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 5544b82a2e7a..c24d32d3589d 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -78,39 +78,33 @@ static void cpu_v7_spectre_init(void)
case ARM_CPU_PART_CORTEX_A57:
case ARM_CPU_PART_CORTEX_A72: {
struct arm_smccc_res res;
+ int conduit;
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
break;
- switch (psci_ops.conduit) {
- case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- if ((int)res.a0 != 0)
- break;
+ conduit = arm_smccc_1_1(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+
+ if ((int)res.a0 == 0) {
if (processor.switch_mm != cpu_v7_hvc_switch_mm && cpu)
goto bl_error;
- per_cpu(harden_branch_predictor_fn, cpu) =
- call_hvc_arch_workaround_1;
- processor.switch_mm = cpu_v7_hvc_switch_mm;
- spectre_v2_method = "hypervisor";
- break;
-
- case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- if ((int)res.a0 != 0)
+ switch (conduit) {
+ case PSCI_CONDUIT_HVC:
+ per_cpu(harden_branch_predictor_fn, cpu) =
+ call_hvc_arch_workaround_1;
+ processor.switch_mm = cpu_v7_hvc_switch_mm;
+ spectre_v2_method = "hypervisor";
break;
- if (processor.switch_mm != cpu_v7_smc_switch_mm && cpu)
- goto bl_error;
- per_cpu(harden_branch_predictor_fn, cpu) =
- call_smc_arch_workaround_1;
- processor.switch_mm = cpu_v7_smc_switch_mm;
- spectre_v2_method = "firmware";
- break;
-
- default:
- break;
+ case PSCI_CONDUIT_SMC:
+ per_cpu(harden_branch_predictor_fn, cpu) =
+ call_smc_arch_workaround_1;
+ processor.switch_mm = cpu_v7_smc_switch_mm;
+ spectre_v2_method = "firmware";
+ break;
+ default:
+ break;
+ }
}
}
#endif
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index a509e35132d2..6266284cc801 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -234,12 +234,14 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
return;
+ arm_smccc_1_1(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+
+ if ((int)res.a0 < 0)
+ return;
+
switch (psci_ops.conduit) {
case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- if ((int)res.a0 < 0)
- return;
cb = call_hvc_arch_workaround_1;
/* This is a guest, no need to patch KVM vectors */
smccc_start = NULL;
@@ -247,10 +249,6 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
break;
case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_1, &res);
- if ((int)res.a0 < 0)
- return;
cb = call_smc_arch_workaround_1;
smccc_start = __smccc_workaround_1_smc_start;
smccc_end = __smccc_workaround_1_smc_end;
@@ -343,6 +341,8 @@ void __init arm64_enable_wa2_handling(struct alt_instr *alt,
void arm64_set_ssbd_mitigation(bool state)
{
+ int conduit;
+
if (this_cpu_has_cap(ARM64_SSBS)) {
if (state)
asm volatile(SET_PSTATE_SSBS(0));
@@ -351,19 +351,9 @@ void arm64_set_ssbd_mitigation(bool state)
return;
}
- switch (psci_ops.conduit) {
- case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
- break;
-
- case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
- break;
+ conduit = arm_smccc_1_1(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
- default:
- WARN_ON_ONCE(1);
- break;
- }
+ WARN_ON_ONCE(conduit == PSCI_CONDUIT_NONE);
}
static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
@@ -385,21 +375,8 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
return false;
}
- switch (psci_ops.conduit) {
- case PSCI_CONDUIT_HVC:
- arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_2, &res);
- break;
-
- case PSCI_CONDUIT_SMC:
- arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
- ARM_SMCCC_ARCH_WORKAROUND_2, &res);
- break;
-
- default:
- ssbd_state = ARM64_SSBD_UNKNOWN;
- return false;
- }
+ arm_smccc_1_1(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_2, &res);
val = (s32)res.a0;
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (3 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 04/12] arm/arm64: Make use of the SMCCC 1.1 wrapper Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-12-10 10:39 ` Mark Rutland
2018-11-28 14:45 ` [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting Steven Price
` (8 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
This provides a mechanism for querying which paravirtualized features
are available in this hypervisor.
Also add the header file which defines the ABI for the paravirtualized
clock features we're about to add.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/pvclock-abi.h | 32 ++++++++++++++++++++++++++++
include/kvm/arm_pv.h | 28 ++++++++++++++++++++++++
include/linux/arm-smccc.h | 1 +
virt/kvm/arm/hypercalls.c | 9 ++++++++
4 files changed, 70 insertions(+)
create mode 100644 arch/arm64/include/asm/pvclock-abi.h
create mode 100644 include/kvm/arm_pv.h
diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h
new file mode 100644
index 000000000000..64ce041c8922
--- /dev/null
+++ b/arch/arm64/include/asm/pvclock-abi.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 Arm Ltd. */
+
+#ifndef __ASM_PVCLOCK_ABI_H
+#define __ASM_PVCLOCK_ABI_H
+
+#include <kvm/arm_pv.h>
+
+struct pvclock_vm_time_info {
+ __le32 revision;
+ __le32 attributes;
+ __le64 sequence_number;
+ __le64 scale_mult;
+ __le32 shift;
+ __le32 reserved;
+ __le64 native_freq;
+ __le64 pv_freq;
+ __le64 div_by_pv_freq_mult;
+} __packed;
+
+struct pvclock_vcpu_stolen_time_info {
+ __le32 revision;
+ __le32 attributes;
+ __le64 stolen_time;
+ /* Structure must be 64 byte aligned, pad to that size */
+ u8 padding[48];
+} __packed;
+
+#define PV_VM_TIME_NOT_SUPPORTED -1
+#define PV_VM_TIME_INVALID_PARAMETERS -2
+
+#endif
diff --git a/include/kvm/arm_pv.h b/include/kvm/arm_pv.h
new file mode 100644
index 000000000000..19d2dafff31a
--- /dev/null
+++ b/include/kvm/arm_pv.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Copyright (C) 2018 Arm Ltd.
+ */
+
+#ifndef __KVM_ARM_PV_H
+#define __KVM_ARM_PV_H
+
+#include <linux/arm-smccc.h>
+
+#define ARM_SMCCC_HV_PV_FEATURES \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_64, \
+ ARM_SMCCC_OWNER_HYP_STANDARD, \
+ 0x20)
+
+#define ARM_SMCCC_HV_PV_TIME_LPT \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_64, \
+ ARM_SMCCC_OWNER_HYP_STANDARD, \
+ 0x21)
+
+#define ARM_SMCCC_HV_PV_TIME_ST \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_64, \
+ ARM_SMCCC_OWNER_HYP_STANDARD, \
+ 0x22)
+
+#endif /* __KVM_ARM_PV_H */
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index b047009e7a0a..4e0866cc48c0 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -54,6 +54,7 @@
#define ARM_SMCCC_OWNER_SIP 2
#define ARM_SMCCC_OWNER_OEM 3
#define ARM_SMCCC_OWNER_STANDARD 4
+#define ARM_SMCCC_OWNER_HYP_STANDARD 5
#define ARM_SMCCC_OWNER_TRUSTED_APP 48
#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
#define ARM_SMCCC_OWNER_TRUSTED_OS 50
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
index 153aa7642100..ba13b798f0f8 100644
--- a/virt/kvm/arm/hypercalls.c
+++ b/virt/kvm/arm/hypercalls.c
@@ -5,6 +5,7 @@
#include <linux/kvm_host.h>
#include <asm/kvm_emulate.h>
+#include <asm/pvclock-abi.h>
#include <kvm/arm_hypercalls.h>
#include <kvm/arm_psci.h>
@@ -40,6 +41,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
break;
}
break;
+ case ARM_SMCCC_HV_PV_FEATURES:
+ val = SMCCC_RET_SUCCESS;
+ break;
+ }
+ break;
+ case ARM_SMCCC_HV_PV_FEATURES:
+ feature = smccc_get_arg1(vcpu);
+ switch (feature) {
}
break;
default:
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call
2018-11-28 14:45 ` [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call Steven Price
@ 2018-12-10 10:39 ` Mark Rutland
2018-12-10 14:20 ` Steven Price
0 siblings, 1 reply; 32+ messages in thread
From: Mark Rutland @ 2018-12-10 10:39 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:20PM +0000, Steven Price wrote:
> This provides a mechanism for querying which paravirtualized features
> are available in this hypervisor.
>
> Also add the header file which defines the ABI for the paravirtualized
> clock features we're about to add.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/include/asm/pvclock-abi.h | 32 ++++++++++++++++++++++++++++
> include/kvm/arm_pv.h | 28 ++++++++++++++++++++++++
> include/linux/arm-smccc.h | 1 +
> virt/kvm/arm/hypercalls.c | 9 ++++++++
> 4 files changed, 70 insertions(+)
> create mode 100644 arch/arm64/include/asm/pvclock-abi.h
> create mode 100644 include/kvm/arm_pv.h
>
> diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h
> new file mode 100644
> index 000000000000..64ce041c8922
> --- /dev/null
> +++ b/arch/arm64/include/asm/pvclock-abi.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2018 Arm Ltd. */
> +
> +#ifndef __ASM_PVCLOCK_ABI_H
> +#define __ASM_PVCLOCK_ABI_H
> +
> +#include <kvm/arm_pv.h>
> +
> +struct pvclock_vm_time_info {
> + __le32 revision;
> + __le32 attributes;
> + __le64 sequence_number;
> + __le64 scale_mult;
> + __le32 shift;
> + __le32 reserved;
> + __le64 native_freq;
> + __le64 pv_freq;
> + __le64 div_by_pv_freq_mult;
> +} __packed;
> +
> +struct pvclock_vcpu_stolen_time_info {
> + __le32 revision;
> + __le32 attributes;
> + __le64 stolen_time;
> + /* Structure must be 64 byte aligned, pad to that size */
> + u8 padding[48];
> +} __packed;
> +
> +#define PV_VM_TIME_NOT_SUPPORTED -1
> +#define PV_VM_TIME_INVALID_PARAMETERS -2
Could you please add a comment describing that these are defined in ARM
DEN0057A?
> +
> +#endif
> diff --git a/include/kvm/arm_pv.h b/include/kvm/arm_pv.h
> new file mode 100644
> index 000000000000..19d2dafff31a
> --- /dev/null
> +++ b/include/kvm/arm_pv.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + * Copyright (C) 2018 Arm Ltd.
> + */
> +
> +#ifndef __KVM_ARM_PV_H
> +#define __KVM_ARM_PV_H
> +
> +#include <linux/arm-smccc.h>
> +
> +#define ARM_SMCCC_HV_PV_FEATURES \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_HYP_STANDARD, \
> + 0x20)
> +
> +#define ARM_SMCCC_HV_PV_TIME_LPT \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_HYP_STANDARD, \
> + 0x21)
> +
> +#define ARM_SMCCC_HV_PV_TIME_ST \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_HYP_STANDARD, \
> + 0x22)
> +
> +#endif /* __KVM_ARM_PV_H */
Do these need to live in a separate header, away from the struct
definitions?
I'd be happy for these to live in <linux/arm-smccc.h>, given they're
standard calls.
As before, a comment referring to ARM DEN0057A would be nice.
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index b047009e7a0a..4e0866cc48c0 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -54,6 +54,7 @@
> #define ARM_SMCCC_OWNER_SIP 2
> #define ARM_SMCCC_OWNER_OEM 3
> #define ARM_SMCCC_OWNER_STANDARD 4
> +#define ARM_SMCCC_OWNER_HYP_STANDARD 5
Minor nit, but could we make that STANDARD_HYP?
> #define ARM_SMCCC_OWNER_TRUSTED_APP 48
> #define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
> #define ARM_SMCCC_OWNER_TRUSTED_OS 50
> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
> index 153aa7642100..ba13b798f0f8 100644
> --- a/virt/kvm/arm/hypercalls.c
> +++ b/virt/kvm/arm/hypercalls.c
> @@ -5,6 +5,7 @@
> #include <linux/kvm_host.h>
>
> #include <asm/kvm_emulate.h>
> +#include <asm/pvclock-abi.h>
>
> #include <kvm/arm_hypercalls.h>
> #include <kvm/arm_psci.h>
> @@ -40,6 +41,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> break;
> }
> break;
> + case ARM_SMCCC_HV_PV_FEATURES:
> + val = SMCCC_RET_SUCCESS;
> + break;
> + }
> + break;
> + case ARM_SMCCC_HV_PV_FEATURES:
> + feature = smccc_get_arg1(vcpu);
> + switch (feature) {
> }
IIUC, at this point in time, this happens to always return
SMCCC_RET_NOT_SUPPORTED.
If you leave this part out of the patch, and add it as required, this
patch is purely adding definitions, which would be a bit nicer for
review.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call
2018-12-10 10:39 ` Mark Rutland
@ 2018-12-10 14:20 ` Steven Price
0 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-12-10 14:20 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On 10/12/2018 10:39, Mark Rutland wrote:
> On Wed, Nov 28, 2018 at 02:45:20PM +0000, Steven Price wrote:
>> This provides a mechanism for querying which paravirtualized features
>> are available in this hypervisor.
>>
>> Also add the header file which defines the ABI for the paravirtualized
>> clock features we're about to add.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> arch/arm64/include/asm/pvclock-abi.h | 32 ++++++++++++++++++++++++++++
>> include/kvm/arm_pv.h | 28 ++++++++++++++++++++++++
>> include/linux/arm-smccc.h | 1 +
>> virt/kvm/arm/hypercalls.c | 9 ++++++++
>> 4 files changed, 70 insertions(+)
>> create mode 100644 arch/arm64/include/asm/pvclock-abi.h
>> create mode 100644 include/kvm/arm_pv.h
>>
>> diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h
>> new file mode 100644
>> index 000000000000..64ce041c8922
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/pvclock-abi.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (C) 2018 Arm Ltd. */
>> +
>> +#ifndef __ASM_PVCLOCK_ABI_H
>> +#define __ASM_PVCLOCK_ABI_H
>> +
>> +#include <kvm/arm_pv.h>
>> +
>> +struct pvclock_vm_time_info {
>> + __le32 revision;
>> + __le32 attributes;
>> + __le64 sequence_number;
>> + __le64 scale_mult;
>> + __le32 shift;
>> + __le32 reserved;
>> + __le64 native_freq;
>> + __le64 pv_freq;
>> + __le64 div_by_pv_freq_mult;
>> +} __packed;
>> +
>> +struct pvclock_vcpu_stolen_time_info {
>> + __le32 revision;
>> + __le32 attributes;
>> + __le64 stolen_time;
>> + /* Structure must be 64 byte aligned, pad to that size */
>> + u8 padding[48];
>> +} __packed;
>> +
>> +#define PV_VM_TIME_NOT_SUPPORTED -1
>> +#define PV_VM_TIME_INVALID_PARAMETERS -2
>
> Could you please add a comment describing that these are defined in ARM
> DEN0057A?
No problem.
>> +
>> +#endif
>> diff --git a/include/kvm/arm_pv.h b/include/kvm/arm_pv.h
>> new file mode 100644
>> index 000000000000..19d2dafff31a
>> --- /dev/null
>> +++ b/include/kvm/arm_pv.h
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + * Copyright (C) 2018 Arm Ltd.
>> + */
>> +
>> +#ifndef __KVM_ARM_PV_H
>> +#define __KVM_ARM_PV_H
>> +
>> +#include <linux/arm-smccc.h>
>> +
>> +#define ARM_SMCCC_HV_PV_FEATURES \
>> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
>> + ARM_SMCCC_SMC_64, \
>> + ARM_SMCCC_OWNER_HYP_STANDARD, \
>> + 0x20)
>> +
>> +#define ARM_SMCCC_HV_PV_TIME_LPT \
>> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
>> + ARM_SMCCC_SMC_64, \
>> + ARM_SMCCC_OWNER_HYP_STANDARD, \
>> + 0x21)
>> +
>> +#define ARM_SMCCC_HV_PV_TIME_ST \
>> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
>> + ARM_SMCCC_SMC_64, \
>> + ARM_SMCCC_OWNER_HYP_STANDARD, \
>> + 0x22)
>> +
>> +#endif /* __KVM_ARM_PV_H */
>
> Do these need to live in a separate header, away from the struct
> definitions?
>
> I'd be happy for these to live in <linux/arm-smccc.h>, given they're
> standard calls.
I'll move them to linux/arm-smccc.h - I didn't want to place them in
pvclock-abi.h as it seemed wrong to pull that into the generic SMCCC
code which doesn't care about these structures.
> As before, a comment referring to ARM DEN0057A would be nice.
Will add
>> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
>> index b047009e7a0a..4e0866cc48c0 100644
>> --- a/include/linux/arm-smccc.h
>> +++ b/include/linux/arm-smccc.h
>> @@ -54,6 +54,7 @@
>> #define ARM_SMCCC_OWNER_SIP 2
>> #define ARM_SMCCC_OWNER_OEM 3
>> #define ARM_SMCCC_OWNER_STANDARD 4
>> +#define ARM_SMCCC_OWNER_HYP_STANDARD 5
>
> Minor nit, but could we make that STANDARD_HYP?
Sure
>> #define ARM_SMCCC_OWNER_TRUSTED_APP 48
>> #define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
>> #define ARM_SMCCC_OWNER_TRUSTED_OS 50
>> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
>> index 153aa7642100..ba13b798f0f8 100644
>> --- a/virt/kvm/arm/hypercalls.c
>> +++ b/virt/kvm/arm/hypercalls.c
>> @@ -5,6 +5,7 @@
>> #include <linux/kvm_host.h>
>>
>> #include <asm/kvm_emulate.h>
>> +#include <asm/pvclock-abi.h>
>>
>> #include <kvm/arm_hypercalls.h>
>> #include <kvm/arm_psci.h>
>> @@ -40,6 +41,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>> break;
>> }
>> break;
>> + case ARM_SMCCC_HV_PV_FEATURES:
>> + val = SMCCC_RET_SUCCESS;
>> + break;
>> + }
>> + break;
>> + case ARM_SMCCC_HV_PV_FEATURES:
>> + feature = smccc_get_arg1(vcpu);
>> + switch (feature) {
>> }
>
> IIUC, at this point in time, this happens to always return
> SMCCC_RET_NOT_SUPPORTED.
Yes, this is because on an oddity in the specification that I'm tempted
to ignore. I originally had PV_FEATURES (only) in the switch in this
patch. However the specification states:
If PV_func_id identifies PV_FEATURES this function can return:
• NOT_SUPPORTED (-1) to indicate that all functions in this
specification are not supported.
• SUCCESS (0) to indicate that one or more
paravirtualization functions are supported.
Since by this patch we haven't reached "one or more" functions a strict
reading of the specification says that even PV_FEATURES should be
returning NOT_SUPPORTED.
> If you leave this part out of the patch, and add it as required, this
> patch is purely adding definitions, which would be a bit nicer for
> review.
Before getting lost in the above wording the specification I had tried
to make the LPT and stolen time patches not dependent on each other.
Given your other comments (in reply to the cover letter), I think I'll
merge this chunk with the first stolen time patch and put all the stolen
time patches first.
Thanks,
Steve
> Thanks,
> Mark.
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (4 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-12-10 10:56 ` Mark Rutland
2018-11-28 14:45 ` [PATCH 07/12] clocksource: arm_arch_timer: Use paravirtualized LPT Steven Price
` (7 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Provide a method for a guest to derive a paravirtualized counter/timer
which isn't dependent on the host's counter frequency. This allows a
guest to be migrated onto a new host which doesn't have the same
frequency without the virtual counter being disturbed.
The host provides a shared page which contains coefficients that can be
used to map the real counter from the host (the Arm "virtual counter")
to a paravirtualized view of time. On migration the new host updates the
coefficients to ensure that the guests view of time (after using the
coefficients) doesn't change and that the derived counter progresses at
the same real frequency.
The guest can probe the existence of this support using the PV_FEATURES
SMCCC interface provided in the previous patch.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/kvm_host.h | 7 ++
include/kvm/arm_arch_timer.h | 2 +
include/linux/kvm_types.h | 2 +
virt/kvm/arm/arm.c | 5 +
virt/kvm/arm/hypercalls.c | 146 ++++++++++++++++++++++++++++++
5 files changed, 162 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 52fbc823ff8c..827162b1fabf 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -80,6 +80,13 @@ struct kvm_arch {
/* Mandated version of PSCI */
u32 psci_version;
+
+ struct kvm_arch_pvtime {
+ void *pv_page;
+
+ gpa_t lpt_page;
+ u32 lpt_fpv;
+ } pvtime;
};
#define KVM_NR_MEM_OBJS 40
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 6502feb9524b..c8cdd96052e0 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -92,6 +92,8 @@ void kvm_timer_init_vhe(void);
bool kvm_arch_timer_get_input_level(int vintid);
+int kvm_arm_update_lpt_sequence(struct kvm *kvm);
+
#define vcpu_vtimer(v) (&(v)->arch.timer_cpu.vtimer)
#define vcpu_ptimer(v) (&(v)->arch.timer_cpu.ptimer)
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 8bf259dae9f6..fd3a2caabeb2 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -49,6 +49,8 @@ typedef unsigned long gva_t;
typedef u64 gpa_t;
typedef u64 gfn_t;
+#define GPA_INVALID -1
+
typedef unsigned long hva_t;
typedef u64 hpa_t;
typedef u64 hfn_t;
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 23774970c9df..4c6355f21352 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -148,6 +148,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm->arch.max_vcpus = vgic_present ?
kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
+ /* Set the PV Time addresses to invalid values */
+ kvm->arch.pvtime.lpt_page = GPA_INVALID;
+
return ret;
out_free_stage2_pgd:
kvm_free_stage2_pgd(kvm);
@@ -587,6 +590,8 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
ret = kvm_arm_pmu_v3_enable(vcpu);
+ kvm_arm_update_lpt_sequence(kvm);
+
return ret;
}
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
index ba13b798f0f8..fdb1880ab4c6 100644
--- a/virt/kvm/arm/hypercalls.c
+++ b/virt/kvm/arm/hypercalls.c
@@ -2,6 +2,7 @@
// Copyright (C) 2018 Arm Ltd.
#include <linux/arm-smccc.h>
+#include <linux/highmem.h>
#include <linux/kvm_host.h>
#include <asm/kvm_emulate.h>
@@ -10,6 +11,145 @@
#include <kvm/arm_hypercalls.h>
#include <kvm/arm_psci.h>
+#include <clocksource/arm_arch_timer.h>
+
+/*
+ * Returns ((u128)dividend << 64) / divisor
+ * Precondition: dividend < divisor
+ */
+static u64 shift64_div(u32 dividend, u32 divisor)
+{
+ u64 high = (u64)dividend << 32;
+ u64 low;
+ u64 rem;
+
+ WARN_ON(dividend >= divisor);
+
+ rem = do_div(high, divisor);
+ low = rem << 32;
+ do_div(low, divisor);
+
+ return (high << 32) | low;
+}
+
+/*
+ * Calculate the relative offset of each vCPU's timer and convert that to the
+ * new timer rate.
+ */
+static void update_vtimer_cval(struct kvm *kvm, u32 previous_rate)
+{
+ u32 current_rate = arch_timer_get_rate();
+ u64 current_time = kvm_phys_timer_read();
+ int i;
+ struct kvm_vcpu *vcpu;
+ u64 rel_cval;
+
+ /* Early out if there's nothing to do */
+ if (likely(previous_rate == current_rate))
+ return;
+
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
+ u64 cntvct;
+ u64 new_cntvct;
+
+ /*
+ * The vtimer should not be already loaded as this function is
+ * only called on the first run of the first VCPU before any
+ * timers are loaded.
+ */
+ if (WARN_ON(vtimer->loaded))
+ continue;
+
+ cntvct = current_time - vtimer->cntvoff;
+ new_cntvct = mul_u64_u32_div(cntvct, current_rate,
+ previous_rate);
+ vtimer->cntvoff = current_time - new_cntvct;
+
+ rel_cval = vtimer->cnt_cval - cntvct;
+
+ rel_cval = mul_u64_u32_div(rel_cval, current_rate,
+ previous_rate);
+
+ vtimer->cnt_cval = new_cntvct + rel_cval;
+ }
+}
+
+int kvm_arm_update_lpt_sequence(struct kvm *kvm)
+{
+ struct pvclock_vm_time_info *pvclock;
+ u64 lpt_ipa = kvm->arch.pvtime.lpt_page;
+ u64 native_freq, pv_freq, scale_mult, div_by_pv_freq_mult;
+ u64 shift = 0;
+ u64 sequence_number = 0;
+
+ if (lpt_ipa == GPA_INVALID)
+ return -EINVAL;
+
+ /* Page address must be 64 byte aligned */
+ if (lpt_ipa & 63)
+ return -EINVAL;
+
+ pvclock = kvm->arch.pvtime.pv_page;
+
+
+ if (!pvclock)
+ return -EINVAL;
+
+ mutex_lock(&kvm->lock);
+
+ sequence_number = le64_to_cpu(pvclock->sequence_number);
+ native_freq = le64_to_cpu(pvclock->native_freq);
+
+ if (native_freq) {
+ /*
+ * The VM has been migrated, so update the sequence number
+ * and correct the compare for the timer if the frequency has
+ * changed
+ */
+ sequence_number = sequence_number + 2;
+ update_vtimer_cval(kvm, native_freq);
+ }
+
+ native_freq = arch_timer_get_rate();
+ pv_freq = kvm->arch.pvtime.lpt_fpv;
+
+ if (pv_freq >= native_freq)
+ shift = ilog2(pv_freq / native_freq) + 1;
+
+ WARN_ON(native_freq > U32_MAX);
+ /* scale_mult = (pv_freq << 64) / (native_freq << shift) */
+ scale_mult = shift64_div(pv_freq, native_freq << shift);
+ /* div_by_pv_freq_mult = (1 << 64) / pv_freq */
+ div_by_pv_freq_mult = shift64_div(1, pv_freq);
+
+ pvclock->sequence_number = cpu_to_le64(sequence_number);
+ pvclock->native_freq = cpu_to_le64(native_freq);
+ pvclock->pv_freq = cpu_to_le64(pv_freq);
+ pvclock->shift = cpu_to_le32(shift);
+ pvclock->scale_mult = cpu_to_le64(scale_mult);
+ pvclock->div_by_pv_freq_mult = cpu_to_le64(div_by_pv_freq_mult);
+
+ mutex_unlock(&kvm->lock);
+
+ return 0;
+}
+
+static int kvm_hypercall_time_lpt(struct kvm_vcpu *vcpu)
+{
+ u32 flags;
+ u64 ret = vcpu->kvm->arch.pvtime.lpt_page;
+
+ flags = smccc_get_arg1(vcpu);
+
+ if (flags) {
+ /* Currently no support for any flags */
+ ret = PV_VM_TIME_INVALID_PARAMETERS;
+ }
+
+ smccc_set_retval(vcpu, ret, 0, 0, 0);
+ return 1;
+}
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
{
u32 func_id = smccc_get_function(vcpu);
@@ -49,8 +189,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
case ARM_SMCCC_HV_PV_FEATURES:
feature = smccc_get_arg1(vcpu);
switch (feature) {
+ case ARM_SMCCC_HV_PV_FEATURES:
+ case ARM_SMCCC_HV_PV_TIME_LPT:
+ val = SMCCC_RET_SUCCESS;
+ break;
}
break;
+ case ARM_SMCCC_HV_PV_TIME_LPT:
+ return kvm_hypercall_time_lpt(vcpu);
default:
return kvm_psci_call(vcpu);
}
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting
2018-11-28 14:45 ` [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting Steven Price
@ 2018-12-10 10:56 ` Mark Rutland
2018-12-10 15:45 ` Steven Price
0 siblings, 1 reply; 32+ messages in thread
From: Mark Rutland @ 2018-12-10 10:56 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:21PM +0000, Steven Price wrote:
> Provide a method for a guest to derive a paravirtualized counter/timer
> which isn't dependent on the host's counter frequency. This allows a
> guest to be migrated onto a new host which doesn't have the same
> frequency without the virtual counter being disturbed.
I have a number of concerns about paravirtualizing the timer frequency,
but I'll bring that up in reply to the cover letter.
I have some orthogonal comments below.
> The host provides a shared page which contains coefficients that can be
> used to map the real counter from the host (the Arm "virtual counter")
> to a paravirtualized view of time. On migration the new host updates the
> coefficients to ensure that the guests view of time (after using the
> coefficients) doesn't change and that the derived counter progresses at
> the same real frequency.
Can we please avoid using the term 'page' here?
There is a data structure in shared memory, but it is not page-sized,
and referring to it as a page here and elsewhere is confusing. The spec
never uses the term 'page'
Could we please say something like:
The host provides a datastrucutre in shared memory which ...
... to avoid the implication this is page sized/aligned etc.
[...]
> + struct kvm_arch_pvtime {
> + void *pv_page;
> +
> + gpa_t lpt_page;
> + u32 lpt_fpv;
> + } pvtime;
To remove the page terminology, perhaps something like:
struct kvm_arch_pvtime {
struct lpt *lpt;
gpa_t lpt_gpa;
u32 lpt_fpv;
};
[...]
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index 8bf259dae9f6..fd3a2caabeb2 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -49,6 +49,8 @@ typedef unsigned long gva_t;
> typedef u64 gpa_t;
> typedef u64 gfn_t;
>
> +#define GPA_INVALID -1
To avoid any fun with signed/unsigned comparison, can we please make
this:
#define GPA_INVALID ((gpa_t)-1)
... or:
#define GPA_INVALID (~(gpa_t)0)
[...]
> +static void update_vtimer_cval(struct kvm *kvm, u32 previous_rate)
> +{
> + u32 current_rate = arch_timer_get_rate();
> + u64 current_time = kvm_phys_timer_read();
> + int i;
> + struct kvm_vcpu *vcpu;
> + u64 rel_cval;
> +
> + /* Early out if there's nothing to do */
> + if (likely(previous_rate == current_rate))
> + return;
Given this only happens on migration, I don't think we need to care
about likely/unlikely here, and can drop that from the condition.
[...]
> +int kvm_arm_update_lpt_sequence(struct kvm *kvm)
> +{
> + struct pvclock_vm_time_info *pvclock;
> + u64 lpt_ipa = kvm->arch.pvtime.lpt_page;
> + u64 native_freq, pv_freq, scale_mult, div_by_pv_freq_mult;
> + u64 shift = 0;
> + u64 sequence_number = 0;
> +
> + if (lpt_ipa == GPA_INVALID)
> + return -EINVAL;
> +
> + /* Page address must be 64 byte aligned */
> + if (lpt_ipa & 63)
> + return -EINVAL;
Please use IS_ALIGNED(), e.g.
if (!IS_ALIGNED(lpt_ipa, 64))
return -EINVAL;
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting
2018-12-10 10:56 ` Mark Rutland
@ 2018-12-10 15:45 ` Steven Price
0 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-12-10 15:45 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On 10/12/2018 10:56, Mark Rutland wrote:
> On Wed, Nov 28, 2018 at 02:45:21PM +0000, Steven Price wrote:
>> Provide a method for a guest to derive a paravirtualized counter/timer
>> which isn't dependent on the host's counter frequency. This allows a
>> guest to be migrated onto a new host which doesn't have the same
>> frequency without the virtual counter being disturbed.
>
> I have a number of concerns about paravirtualizing the timer frequency,
> but I'll bring that up in reply to the cover letter.
>
> I have some orthogonal comments below.
>
>> The host provides a shared page which contains coefficients that can be
>> used to map the real counter from the host (the Arm "virtual counter")
>> to a paravirtualized view of time. On migration the new host updates the
>> coefficients to ensure that the guests view of time (after using the
>> coefficients) doesn't change and that the derived counter progresses at
>> the same real frequency.
>
> Can we please avoid using the term 'page' here?
>
> There is a data structure in shared memory, but it is not page-sized,
> and referring to it as a page here and elsewhere is confusing. The spec
> never uses the term 'page'
>
> Could we please say something like:
>
> The host provides a datastrucutre in shared memory which ...
>
> ... to avoid the implication this is page sized/aligned etc.
>
> [...]
Sure, I'll update to avoid referring to it as a page. Although note that
when mapping it into the guest we can obviously only map in page sized
granules, so in practise the LPT structure is contained within a entire
page given to the guest...
>> + struct kvm_arch_pvtime {
>> + void *pv_page;
>> +
>> + gpa_t lpt_page;
>> + u32 lpt_fpv;
>> + } pvtime;
>
> To remove the page terminology, perhaps something like:
>
> struct kvm_arch_pvtime {
> struct lpt *lpt;
> gpa_t lpt_gpa;
> u32 lpt_fpv;
> };
>
> [...]
>
>> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
>> index 8bf259dae9f6..fd3a2caabeb2 100644
>> --- a/include/linux/kvm_types.h
>> +++ b/include/linux/kvm_types.h
>> @@ -49,6 +49,8 @@ typedef unsigned long gva_t;
>> typedef u64 gpa_t;
>> typedef u64 gfn_t;
>>
>> +#define GPA_INVALID -1
>
> To avoid any fun with signed/unsigned comparison, can we please make
> this:
>
> #define GPA_INVALID ((gpa_t)-1)
>
> ... or:
>
> #define GPA_INVALID (~(gpa_t)0)
>
> [...]
I'll go with the latter as I think that's clearer.
>> +static void update_vtimer_cval(struct kvm *kvm, u32 previous_rate)
>> +{
>> + u32 current_rate = arch_timer_get_rate();
>> + u64 current_time = kvm_phys_timer_read();
>> + int i;
>> + struct kvm_vcpu *vcpu;
>> + u64 rel_cval;
>> +
>> + /* Early out if there's nothing to do */
>> + if (likely(previous_rate == current_rate))
>> + return;
>
> Given this only happens on migration, I don't think we need to care
> about likely/unlikely here, and can drop that from the condition.
Fair enough
> [...]
>
>> +int kvm_arm_update_lpt_sequence(struct kvm *kvm)
>> +{
>> + struct pvclock_vm_time_info *pvclock;
>> + u64 lpt_ipa = kvm->arch.pvtime.lpt_page;
>> + u64 native_freq, pv_freq, scale_mult, div_by_pv_freq_mult;
>> + u64 shift = 0;
>> + u64 sequence_number = 0;
>> +
>> + if (lpt_ipa == GPA_INVALID)
>> + return -EINVAL;
>> +
>> + /* Page address must be 64 byte aligned */
>> + if (lpt_ipa & 63)
>> + return -EINVAL;
>
> Please use IS_ALIGNED(), e.g.
>
> if (!IS_ALIGNED(lpt_ipa, 64))
> return -EINVAL;
Yes, much clearer - no need for the comment :)
Thanks,
Steve
> Thanks,
> Mark.
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 07/12] clocksource: arm_arch_timer: Use paravirtualized LPT
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (5 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-11-28 14:45 ` [PATCH 08/12] KVM: Export mark_page_dirty_in_slot Steven Price
` (6 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Enable paravirtualized time to be used in a KVM guest if the host
supports it. This allows the guest to derive a counter which is clocked
at a persistent rate even when the guest is migrated.
If we discover that the system supports SMCCC v1.1 then we probe to
determine whether the hypervisor supports paravirtualized features and
finally whether it supports "Live Physical Time" reporting. If so a
shared structure is made available to the guest containing coefficients
to calculate the derived clock.
The guest kernel uses the coefficients to present a clock to user space
that is always clocked at the same rate whenever the guest is running
('live'), even if the physical clock changes (due to the guest being
migrated).
The existing workaround framework for CNTVCT is used to disable the VDSO
and trap user space accesses to the timer registers so we can present the
derived clock.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/arch_timer.h | 32 ++++-
arch/arm64/kernel/cpuinfo.c | 2 +-
drivers/clocksource/arm_arch_timer.c | 176 ++++++++++++++++++++++++++-
3 files changed, 204 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index f2a234d6516c..ec0e7250c453 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -20,12 +20,14 @@
#define __ASM_ARCH_TIMER_H
#include <asm/barrier.h>
+#include <asm/pvclock-abi.h>
#include <asm/sysreg.h>
#include <linux/bug.h>
#include <linux/init.h>
#include <linux/jump_label.h>
#include <linux/smp.h>
+#include <linux/static_key.h>
#include <linux/types.h>
#include <clocksource/arm_arch_timer.h>
@@ -79,6 +81,19 @@ DECLARE_PER_CPU(const struct arch_timer_erratum_workaround *,
_val; \
})
+void pvclock_reg_write_cntv_tval_el0(u32 val);
+extern struct static_key_false arch_counter_cntfrq_ool_enabled;
+extern u64 pvclock_get_cntfrq(void);
+extern struct static_key_false arch_counter_cntvct_ool_enabled;
+extern u64 pvclock_get_cntvct(void);
+
+static __always_inline void __write_cntv_tval_el0(u32 val)
+{
+ if (static_branch_unlikely(&arch_counter_cntvct_ool_enabled))
+ return pvclock_reg_write_cntv_tval_el0(val);
+ write_sysreg(val, cntv_tval_el0);
+}
+
/*
* These register accessors are marked inline so the compiler can
* nicely work out which register we want, and chuck away the rest of
@@ -102,7 +117,7 @@ void arch_timer_reg_write_cp15(int access, enum arch_timer_reg reg, u32 val)
write_sysreg(val, cntv_ctl_el0);
break;
case ARCH_TIMER_REG_TVAL:
- write_sysreg(val, cntv_tval_el0);
+ __write_cntv_tval_el0(val);
break;
}
}
@@ -134,7 +149,10 @@ u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
static inline u32 arch_timer_get_cntfrq(void)
{
- return read_sysreg(cntfrq_el0);
+ if (static_branch_unlikely(&arch_counter_cntfrq_ool_enabled))
+ return pvclock_get_cntfrq();
+ else
+ return read_sysreg(cntfrq_el0);
}
static inline u32 arch_timer_get_cntkctl(void)
@@ -154,12 +172,20 @@ static inline u64 arch_counter_get_cntpct(void)
return arch_timer_reg_read_stable(cntpct_el0);
}
-static inline u64 arch_counter_get_cntvct(void)
+static inline u64 __arch_counter_get_cntvct(void)
{
isb();
return arch_timer_reg_read_stable(cntvct_el0);
}
+static inline u64 arch_counter_get_cntvct(void)
+{
+ if (static_branch_unlikely(&arch_counter_cntvct_ool_enabled))
+ return pvclock_get_cntvct();
+ else
+ return __arch_counter_get_cntvct();
+}
+
static inline int arch_timer_arch_init(void)
{
return 0;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index bcc2831399cb..74410727829d 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -324,7 +324,7 @@ static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
{
- info->reg_cntfrq = arch_timer_get_cntfrq();
+ info->reg_cntfrq = read_cpuid(CNTFRQ_EL0);
/*
* Use the effective value of the CTR_EL0 than the raw value
* exposed by the CPU. CTR_E0.IDC field value must be interpreted
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 9a7d4dc00b6e..1991d5044496 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -11,6 +11,7 @@
#define pr_fmt(fmt) "arm_arch_timer: " fmt
+#include <linux/arm-smccc.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/device.h>
@@ -23,6 +24,8 @@
#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/io.h>
+#include <linux/psci.h>
+#include <linux/reboot.h>
#include <linux/slab.h>
#include <linux/sched/clock.h>
#include <linux/sched_clock.h>
@@ -86,6 +89,170 @@ static int __init early_evtstrm_cfg(char *buf)
}
early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg);
+#ifdef CONFIG_ARM64
+/* Paravirtualised time is only supported for 64 bit */
+static struct pvclock_vm_time_info *pvclock_vm_time_info;
+
+DEFINE_STATIC_KEY_FALSE(arch_counter_cntvct_ool_enabled);
+EXPORT_SYMBOL_GPL(arch_counter_cntvct_ool_enabled);
+DEFINE_STATIC_KEY_FALSE(arch_counter_cntfrq_ool_enabled);
+EXPORT_SYMBOL_GPL(arch_counter_cntfrq_ool_enabled);
+
+static inline u64 native_to_pv_cycles(const struct pvclock_vm_time_info *info,
+ u64 cnt)
+{
+ u32 shift = le32_to_cpu(info->shift);
+ u64 scale_mult = le64_to_cpu(info->scale_mult);
+
+ cnt <<= shift;
+ return mul_u64_u64_shr(scale_mult, cnt, 64);
+}
+
+static inline u64 pv_to_native_cycles(const struct pvclock_vm_time_info *info,
+ u64 cnt)
+{
+ u64 native_freq = le64_to_cpu(info->native_freq);
+ u64 pv_freq = le64_to_cpu(info->pv_freq);
+ u64 div_by_pv_freq_mult = le64_to_cpu(info->div_by_pv_freq_mult);
+
+ cnt = native_freq * cnt + pv_freq - 1;
+ return mul_u64_u64_shr(div_by_pv_freq_mult, cnt, 64);
+}
+
+u64 pvclock_get_cntvct(void)
+{
+ u64 cval;
+ __le64 seq_begin, seq_end;
+
+ do {
+ seq_begin = READ_ONCE(pvclock_vm_time_info->sequence_number);
+
+ barrier();
+
+ cval = __arch_counter_get_cntvct();
+ cval = native_to_pv_cycles(pvclock_vm_time_info, cval);
+
+ barrier();
+ seq_end = READ_ONCE(pvclock_vm_time_info->sequence_number);
+ } while (unlikely(seq_begin != seq_end));
+
+ return cval;
+}
+
+u64 pvclock_get_cntfrq(void)
+{
+ return le64_to_cpu(pvclock_vm_time_info->pv_freq);
+}
+
+static void arch_timer_pvclock_init(void)
+{
+ struct arm_smccc_res res;
+ void *kaddr;
+
+ if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
+ return;
+
+ arm_smccc_1_1(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_HV_PV_FEATURES, &res);
+
+ if (res.a0 != SMCCC_RET_SUCCESS)
+ return;
+
+ arm_smccc_1_1(ARM_SMCCC_HV_PV_FEATURES, ARM_SMCCC_HV_PV_TIME_LPT, &res);
+
+ if ((s32)res.a0 < 0)
+ return;
+
+ arm_smccc_1_1(ARM_SMCCC_HV_PV_TIME_LPT, 0, &res);
+
+ if ((s64)res.a0 < 0)
+ return;
+
+ kaddr = memremap(res.a0,
+ sizeof(struct pvclock_vm_time_info),
+ MEMREMAP_WB);
+
+ if (!kaddr) {
+ pr_warn("Failed to map LPT structure for paravirtualized clock\n");
+ return;
+ }
+
+ pvclock_vm_time_info = kaddr;
+
+ static_branch_enable(&arch_counter_cntvct_ool_enabled);
+ static_branch_enable(&arch_counter_cntfrq_ool_enabled);
+
+ pr_info("Using paravirtualized clock\n");
+}
+
+static inline bool pvclock_trap_cntvct(void)
+{
+ return static_branch_unlikely(&arch_counter_cntvct_ool_enabled);
+}
+
+static inline void arch_timer_reg_write_cntv_tval(u32 val,
+ struct arch_timer *timer)
+{
+ __le64 seq_begin, seq_end;
+
+ if (!static_branch_unlikely(&arch_counter_cntvct_ool_enabled)) {
+ writel_relaxed(val, timer->base + CNTV_TVAL);
+ return;
+ }
+
+ do {
+ u32 n_val;
+
+ seq_begin = READ_ONCE(pvclock_vm_time_info->sequence_number);
+
+ barrier();
+
+ n_val = pv_to_native_cycles(pvclock_vm_time_info, val);
+
+ writel_relaxed(n_val, timer->base + CNTV_TVAL);
+ barrier();
+
+ seq_end = READ_ONCE(pvclock_vm_time_info->sequence_number);
+ } while (unlikely(seq_begin != seq_end));
+}
+
+void pvclock_reg_write_cntv_tval_el0(u32 val)
+{
+ __le64 seq_begin, seq_end;
+
+ do {
+ u32 n_val;
+
+ seq_begin = READ_ONCE(pvclock_vm_time_info->sequence_number);
+
+ barrier();
+
+ n_val = pv_to_native_cycles(pvclock_vm_time_info, val);
+
+ write_sysreg(n_val, cntv_tval_el0);
+ barrier();
+
+ seq_end = READ_ONCE(pvclock_vm_time_info->sequence_number);
+ } while (unlikely(seq_begin != seq_end));
+}
+
+#else /* CONFIG_ARM64 */
+static void arch_timer_pvclock_init(void)
+{
+}
+
+static inline bool pvclock_trap_cntvct(void)
+{
+ return false;
+}
+
+static inline void arch_timer_reg_write_cntv_tval(u32 val,
+ struct arch_timer *timer)
+{
+ writel_relaxed(val, timer->base + CNTV_TVAL);
+}
+#endif /* CONFIG_ARM64 */
+
/*
* Architected system timer support.
*/
@@ -111,7 +278,7 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
writel_relaxed(val, timer->base + CNTV_CTL);
break;
case ARCH_TIMER_REG_TVAL:
- writel_relaxed(val, timer->base + CNTV_TVAL);
+ arch_timer_reg_write_cntv_tval(val, timer);
break;
}
} else {
@@ -589,6 +756,7 @@ static bool arch_timer_this_cpu_has_cntvct_wa(void)
#define erratum_set_next_event_tval_phys(...) ({BUG(); 0;})
#define erratum_handler(fn, r, ...) ({false;})
#define arch_timer_this_cpu_has_cntvct_wa() ({false;})
+
#endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
static __always_inline irqreturn_t timer_handler(const int access,
@@ -815,7 +983,7 @@ static void arch_counter_set_user_access(void)
* need to be workaround. The vdso may have been already
* disabled though.
*/
- if (arch_timer_this_cpu_has_cntvct_wa())
+ if (pvclock_trap_cntvct() || arch_timer_this_cpu_has_cntvct_wa())
pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
else
cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
@@ -1222,6 +1390,8 @@ static int __init arch_timer_of_init(struct device_node *np)
arch_timer_kvm_info.virtual_irq = arch_timer_ppi[ARCH_TIMER_VIRT_PPI];
+ arch_timer_pvclock_init();
+
rate = arch_timer_get_cntfrq();
arch_timer_of_configure_rate(rate, np);
@@ -1552,6 +1722,8 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
arch_timer_kvm_info.virtual_irq = arch_timer_ppi[ARCH_TIMER_VIRT_PPI];
+ arch_timer_pvclock_init();
+
/*
* When probing via ACPI, we have no mechanism to override the sysreg
* CNTFRQ value. This *must* be correct.
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 08/12] KVM: Export mark_page_dirty_in_slot
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (6 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 07/12] clocksource: arm_arch_timer: Use paravirtualized LPT Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-11-28 14:45 ` [PATCH 09/12] KVM: arm64: Support stolen time reporting via shared page Steven Price
` (5 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@arm.com>
Callers outside kvm_main.c already holding a reference to the memslot
and the gfn of a page that gets modified by KVM should not have to
lookup the memslot one more time when when going via mark_page_dirty().
Instead, export this function.
Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
include/linux/kvm_host.h | 1 +
virt/kvm/kvm_main.c | 6 ++----
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c926698040e0..c192eda05b95 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -701,6 +701,7 @@ int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len);
struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn);
bool kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
+void mark_page_dirty_in_slot(struct kvm_memory_slot *memslot, gfn_t gfn);
unsigned long kvm_host_page_size(struct kvm *kvm, gfn_t gfn);
void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2679e476b6c3..af38ce961d24 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -127,8 +127,6 @@ static void hardware_disable_all(void);
static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
-static void mark_page_dirty_in_slot(struct kvm_memory_slot *memslot, gfn_t gfn);
-
__visible bool kvm_rebooting;
EXPORT_SYMBOL_GPL(kvm_rebooting);
@@ -2050,8 +2048,7 @@ int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len)
}
EXPORT_SYMBOL_GPL(kvm_clear_guest);
-static void mark_page_dirty_in_slot(struct kvm_memory_slot *memslot,
- gfn_t gfn)
+void mark_page_dirty_in_slot(struct kvm_memory_slot *memslot, gfn_t gfn)
{
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
@@ -2059,6 +2056,7 @@ static void mark_page_dirty_in_slot(struct kvm_memory_slot *memslot,
set_bit_le(rel_gfn, memslot->dirty_bitmap);
}
}
+EXPORT_SYMBOL_GPL(mark_page_dirty_in_slot);
void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
{
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 09/12] KVM: arm64: Support stolen time reporting via shared page
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (7 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 08/12] KVM: Export mark_page_dirty_in_slot Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-11-28 14:45 ` [PATCH 10/12] arm64: Retrieve stolen time as paravirtualized guest Steven Price
` (4 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Implement the service call for configuring a shared page between a VCPU
and the hypervisor in which the hypervisor can write the time stolen
from the VCPU's execution time by other tasks on the host.
We translate the IPA provided by user space (in a later patch) to the
corresponding physical page on the host, which we pin in memory and
kmap to the kernel's linear mapping. We can then use WRITE_ONCE() to
ensure single copy atomicity of the 64-bit unsigned value that reports
stolen time in nanoseconds.
We make sure to update the page mapping if user space changes the
memslots during execution by using the existing gfn_to_hva_cache feature
and memslots generation counter. If the gfn to hva mapping changes, we
update the pinned and mapped page accordingly.
Whenever stolen time is enabled by the guest, the stolen time counter is
reset.
The stolen time itself is retrieved from the sched_info structure
maintained by the Linux scheduler code. We enable SCHEDSTATS when
selecting KVM Kconfig to ensure this value is meaningful.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/kvm_host.h | 9 ++++
arch/arm64/kvm/Kconfig | 1 +
include/kvm/arm_hypercalls.h | 1 +
virt/kvm/arm/arm.c | 20 ++++++++-
virt/kvm/arm/hypercalls.c | 70 +++++++++++++++++++++++++++++++
5 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 827162b1fabf..c6bc1fc8ee00 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -48,6 +48,7 @@
#define KVM_REQ_SLEEP \
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
+#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(2)
DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
@@ -86,6 +87,8 @@ struct kvm_arch {
gpa_t lpt_page;
u32 lpt_fpv;
+
+ gpa_t st_base;
} pvtime;
};
@@ -307,6 +310,12 @@ struct kvm_vcpu_arch {
/* True when deferrable sysregs are loaded on the physical CPU,
* see kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs. */
bool sysregs_loaded_on_cpu;
+
+ /* Guest PV state */
+ struct {
+ u64 steal;
+ u64 last_steal;
+ } steal;
};
/* vcpu_arch flags field values: */
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 47b23bf617c7..92676920d671 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -40,6 +40,7 @@ config KVM
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
select HAVE_KVM_VCPU_RUN_PID_CHANGE
+ select SCHEDSTATS
---help---
Support hosting virtualized guest machines.
We don't support KVM with 16K page tables yet, due to the multiple
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index e5f7f81196b6..2e03e993ad64 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -7,6 +7,7 @@
#include <asm/kvm_emulate.h>
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
+int kvm_update_stolen_time(struct kvm_vcpu *vcpu);
static inline u32 smccc_get_function(struct kvm_vcpu *vcpu)
{
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 4c6355f21352..d4ba21d3e7a5 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -32,8 +32,6 @@
#include <linux/irqbypass.h>
#include <linux/sched/stat.h>
#include <trace/events/kvm.h>
-#include <kvm/arm_pmu.h>
-#include <kvm/arm_psci.h>
#define CREATE_TRACE_POINTS
#include "trace.h"
@@ -52,6 +50,10 @@
#include <asm/kvm_coproc.h>
#include <asm/sections.h>
+#include <kvm/arm_hypercalls.h>
+#include <kvm/arm_pmu.h>
+#include <kvm/arm_psci.h>
+
#ifdef REQUIRES_VIRT
__asm__(".arch_extension virt");
#endif
@@ -150,6 +152,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
/* Set the PV Time addresses to invalid values */
kvm->arch.pvtime.lpt_page = GPA_INVALID;
+ kvm->arch.pvtime.st_base = GPA_INVALID;
return ret;
out_free_stage2_pgd:
@@ -386,6 +389,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
kvm_timer_vcpu_load(vcpu);
kvm_vcpu_load_sysregs(vcpu);
kvm_arch_vcpu_load_fp(vcpu);
+ kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
if (single_task_running())
vcpu_clear_wfe_traps(vcpu);
@@ -634,6 +638,15 @@ static void vcpu_req_sleep(struct kvm_vcpu *vcpu)
}
}
+static void vcpu_req_record_steal(struct kvm_vcpu *vcpu)
+{
+ int idx;
+
+ idx = srcu_read_lock(&vcpu->kvm->srcu);
+ kvm_update_stolen_time(vcpu);
+ srcu_read_unlock(&vcpu->kvm->srcu, idx);
+}
+
static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
{
return vcpu->arch.target >= 0;
@@ -650,6 +663,9 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
* that a VCPU sees new virtual interrupts.
*/
kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
+
+ if (kvm_check_request(KVM_REQ_RECORD_STEAL, vcpu))
+ vcpu_req_record_steal(vcpu);
}
}
diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
index fdb1880ab4c6..513f2285b29a 100644
--- a/virt/kvm/arm/hypercalls.c
+++ b/virt/kvm/arm/hypercalls.c
@@ -150,6 +150,73 @@ static int kvm_hypercall_time_lpt(struct kvm_vcpu *vcpu)
smccc_set_retval(vcpu, ret, 0, 0, 0);
return 1;
}
+
+static struct pvclock_vcpu_stolen_time_info *pvtime_get_st(
+ struct kvm_vcpu *vcpu)
+{
+ void *pv_page = vcpu->kvm->arch.pvtime.pv_page;
+ struct pvclock_vcpu_stolen_time_info *st;
+
+ if (!pv_page)
+ return NULL;
+
+ st = pv_page + PAGE_SIZE;
+
+ return &st[kvm_vcpu_get_idx(vcpu)];
+}
+
+int kvm_update_stolen_time(struct kvm_vcpu *vcpu)
+{
+ u64 steal;
+ struct pvclock_vcpu_stolen_time_info *kaddr;
+
+ if (vcpu->kvm->arch.pvtime.st_base == GPA_INVALID)
+ return -ENOTSUPP;
+
+ kaddr = pvtime_get_st(vcpu);
+
+ if (!kaddr)
+ return -ENOTSUPP;
+
+ kaddr->revision = 0;
+ kaddr->attributes = 0;
+
+ /* Let's do the local bookkeeping */
+ steal = vcpu->arch.steal.steal;
+ steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal;
+ vcpu->arch.steal.last_steal = current->sched_info.run_delay;
+ vcpu->arch.steal.steal = steal;
+
+ /* Now write out the value to the shared page */
+ WRITE_ONCE(kaddr->stolen_time, cpu_to_le64(steal));
+
+ return 0;
+}
+
+static int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
+{
+ u64 ret;
+ int err;
+
+ /*
+ * Start counting stolen time from the time the guest requests
+ * the feature enabled.
+ */
+ vcpu->arch.steal.steal = 0;
+ vcpu->arch.steal.last_steal = current->sched_info.run_delay;
+
+ err = kvm_update_stolen_time(vcpu);
+
+ if (err)
+ ret = SMCCC_RET_NOT_SUPPORTED;
+ else
+ ret = vcpu->kvm->arch.pvtime.st_base +
+ (sizeof(struct pvclock_vcpu_stolen_time_info) *
+ kvm_vcpu_get_idx(vcpu));
+
+ smccc_set_retval(vcpu, ret, 0, 0, 0);
+ return 1;
+}
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
{
u32 func_id = smccc_get_function(vcpu);
@@ -191,12 +258,15 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
switch (feature) {
case ARM_SMCCC_HV_PV_FEATURES:
case ARM_SMCCC_HV_PV_TIME_LPT:
+ case ARM_SMCCC_HV_PV_TIME_ST:
val = SMCCC_RET_SUCCESS;
break;
}
break;
case ARM_SMCCC_HV_PV_TIME_LPT:
return kvm_hypercall_time_lpt(vcpu);
+ case ARM_SMCCC_HV_PV_TIME_ST:
+ return kvm_hypercall_stolen_time(vcpu);
default:
return kvm_psci_call(vcpu);
}
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 10/12] arm64: Retrieve stolen time as paravirtualized guest
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (8 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 09/12] KVM: arm64: Support stolen time reporting via shared page Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-11-28 14:45 ` [PATCH 11/12] KVM: Allow kvm_device_ops to be const Steven Price
` (3 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Enable paravirtualization features when running under a hypervisor
supporting the PV_TIME_ST hypercall.
For each (v)CPU, we ask the hypervisor for the location of a shared
page which the hypervisor will use to report stolen time to us. We set
pv_time_ops to the stolen time function which simply reads the stolen
value from the shared page for a VCPU. We guarantee single-copy
atomicity using READ_ONCE which means we can also read the stolen
time for another VCPU than the currently running one while it is
potentially being updated by the hypervisor.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/kvm.c | 156 +++++++++++++++++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
3 files changed, 158 insertions(+)
create mode 100644 arch/arm64/kernel/kvm.c
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 4c8b13bede80..daaed14ed7a4 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -57,6 +57,7 @@ arm64-obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
arm64-obj-$(CONFIG_CRASH_CORE) += crash_core.o
arm64-obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
arm64-obj-$(CONFIG_ARM64_SSBD) += ssbd.o
+arm64-obj-$(CONFIG_PARAVIRT) += kvm.o
obj-y += $(arm64-obj-y) vdso/ probes/
obj-m += $(arm64-obj-m)
diff --git a/arch/arm64/kernel/kvm.c b/arch/arm64/kernel/kvm.c
new file mode 100644
index 000000000000..72c9554648fc
--- /dev/null
+++ b/arch/arm64/kernel/kvm.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Arm Ltd.
+
+#define pr_fmt(fmt) "kvmarm-pv: " fmt
+
+#include <linux/arm-smccc.h>
+#include <linux/cpuhotplug.h>
+#include <linux/io.h>
+#include <linux/printk.h>
+#include <linux/psci.h>
+#include <linux/reboot.h>
+#include <linux/slab.h>
+
+#include <kvm/arm_pv.h>
+
+#include <asm/paravirt.h>
+#include <asm/smp_plat.h>
+
+struct kvmarm_stolen_time_region {
+ struct pvclock_vcpu_stolen_time_info *kaddr;
+};
+
+static DEFINE_PER_CPU(struct kvmarm_stolen_time_region, stolen_time_region);
+
+static bool steal_acc = true;
+static int __init parse_no_stealacc(char *arg)
+{
+ steal_acc = false;
+ return 0;
+}
+early_param("no-steal-acc", parse_no_stealacc);
+
+/* return stolen time in ns by asking the hypervisor */
+static u64 kvm_steal_clock(int cpu)
+{
+ struct kvmarm_stolen_time_region *reg;
+
+ reg = per_cpu_ptr(&stolen_time_region, cpu);
+ if (!reg->kaddr) {
+ pr_warn_once("stolen time enabled but not configued for cpu %d\n",
+ cpu);
+ return 0;
+ }
+
+ return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time));
+}
+
+static int disable_stolen_time_current_cpu(void)
+{
+ struct kvmarm_stolen_time_region *reg;
+
+ reg = this_cpu_ptr(&stolen_time_region);
+ if (!reg->kaddr)
+ return 0;
+
+ memunmap(reg->kaddr);
+ memset(reg, 0, sizeof(*reg));
+
+ return 0;
+}
+
+static int stolen_time_dying_cpu(unsigned int cpu)
+{
+ return disable_stolen_time_current_cpu();
+}
+
+static int init_stolen_time_cpu(unsigned int cpu)
+{
+ struct kvmarm_stolen_time_region *reg;
+ int ret;
+ struct arm_smccc_res res;
+
+ reg = this_cpu_ptr(&stolen_time_region);
+
+ if (reg->kaddr)
+ return 0;
+
+ ret = arm_smccc_1_1(ARM_SMCCC_HV_PV_TIME_ST, &res);
+
+ if ((long)res.a0 < 0)
+ return -EINVAL;
+
+ reg->kaddr = memremap(res.a0,
+ sizeof(struct pvclock_vcpu_stolen_time_info),
+ MEMREMAP_WB);
+
+ if (reg->kaddr == NULL) {
+ pr_warn("Failed to map stolen time data structure\n");
+ return -EINVAL;
+ }
+
+ if (le32_to_cpu(reg->kaddr->revision) != 0 ||
+ le32_to_cpu(reg->kaddr->attributes) != 0) {
+ pr_warn("Unexpected revision or attributes in stolen time data\n");
+ return -ENXIO;
+ }
+
+ return 0;
+}
+
+static int kvm_arm_init_stolen_time(void)
+{
+ int ret;
+
+ ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING,
+ "hypervisor/kvmarm/pv:starting",
+ init_stolen_time_cpu, stolen_time_dying_cpu);
+ if (ret < 0)
+ return ret;
+ return 0;
+}
+
+static bool has_kvm_steal_clock(void)
+{
+ struct arm_smccc_res res;
+
+ /* To detect the presence of PV time support we require SMCCC 1.1+ */
+ if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
+ return false;
+
+ arm_smccc_1_1(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_HV_PV_FEATURES, &res);
+
+ if (res.a0 != SMCCC_RET_SUCCESS)
+ return false;
+
+ arm_smccc_1_1(ARM_SMCCC_HV_PV_FEATURES, ARM_SMCCC_HV_PV_TIME_ST, &res);
+
+ if (res.a0 != SMCCC_RET_SUCCESS)
+ return false;
+
+ return true;
+}
+
+static int __init kvm_guest_init(void)
+{
+ int ret = 0;
+
+ if (!has_kvm_steal_clock())
+ return 0;
+
+ ret = kvm_arm_init_stolen_time();
+ if (ret)
+ return ret;
+
+ pv_ops.time.steal_clock = kvm_steal_clock;
+
+ static_key_slow_inc(¶virt_steal_enabled);
+ if (steal_acc)
+ static_key_slow_inc(¶virt_steal_rq_enabled);
+
+ pr_info("using stolen time PV\n");
+
+ return 0;
+}
+early_initcall(kvm_guest_init);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index e0cd2baa8380..5c176a31deb8 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -134,6 +134,7 @@ enum cpuhp_state {
/* Must be the last timer callback */
CPUHP_AP_DUMMY_TIMER_STARTING,
CPUHP_AP_ARM_XEN_STARTING,
+ CPUHP_AP_ARM_KVMPV_STARTING,
CPUHP_AP_ARM_CORESIGHT_STARTING,
CPUHP_AP_ARM64_ISNDEP_STARTING,
CPUHP_AP_SMPCFD_DYING,
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 11/12] KVM: Allow kvm_device_ops to be const
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (9 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 10/12] arm64: Retrieve stolen time as paravirtualized guest Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-11-28 14:45 ` [PATCH 12/12] KVM: arm64: Provide a PV_TIME device to user space Steven Price
` (2 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Currently a kvm_device_ops structure cannot be const without triggering
compiler warnings. However the structure doesn't need to be written to
and, by marking it const, it can be read-only in memory. Add some more
const keywords to allow this.
Signed-off-by: Steven Price <steven.price@arm.com>
---
include/linux/kvm_host.h | 4 ++--
virt/kvm/kvm_main.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c192eda05b95..14afddd9e70b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1178,7 +1178,7 @@ extern unsigned int halt_poll_ns_grow;
extern unsigned int halt_poll_ns_shrink;
struct kvm_device {
- struct kvm_device_ops *ops;
+ const struct kvm_device_ops *ops;
struct kvm *kvm;
void *private;
struct list_head vm_node;
@@ -1221,7 +1221,7 @@ struct kvm_device_ops {
void kvm_device_get(struct kvm_device *dev);
void kvm_device_put(struct kvm_device *dev);
struct kvm_device *kvm_device_from_filp(struct file *filp);
-int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type);
+int kvm_register_device_ops(const struct kvm_device_ops *ops, u32 type);
void kvm_unregister_device_ops(u32 type);
extern struct kvm_device_ops kvm_mpic_ops;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index af38ce961d24..c99b3bc85af1 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2853,14 +2853,14 @@ struct kvm_device *kvm_device_from_filp(struct file *filp)
return filp->private_data;
}
-static struct kvm_device_ops *kvm_device_ops_table[KVM_DEV_TYPE_MAX] = {
+static const struct kvm_device_ops *kvm_device_ops_table[KVM_DEV_TYPE_MAX] = {
#ifdef CONFIG_KVM_MPIC
[KVM_DEV_TYPE_FSL_MPIC_20] = &kvm_mpic_ops,
[KVM_DEV_TYPE_FSL_MPIC_42] = &kvm_mpic_ops,
#endif
};
-int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type)
+int kvm_register_device_ops(const struct kvm_device_ops *ops, u32 type)
{
if (type >= ARRAY_SIZE(kvm_device_ops_table))
return -ENOSPC;
@@ -2881,7 +2881,7 @@ void kvm_unregister_device_ops(u32 type)
static int kvm_ioctl_create_device(struct kvm *kvm,
struct kvm_create_device *cd)
{
- struct kvm_device_ops *ops = NULL;
+ const struct kvm_device_ops *ops = NULL;
struct kvm_device *dev;
bool test = cd->flags & KVM_CREATE_DEVICE_TEST;
int ret;
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 12/12] KVM: arm64: Provide a PV_TIME device to user space
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (10 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 11/12] KVM: Allow kvm_device_ops to be const Steven Price
@ 2018-11-28 14:45 ` Steven Price
2018-12-03 13:25 ` [PATCH 00/12] arm64: Paravirtualized time support Andrew Jones
2018-12-10 11:40 ` Mark Rutland
13 siblings, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-11-28 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Allow user space to inform the KVM host where in the physical memory
map the paravirtualized time structures should be located.
A single device is created which provides two base addresses. One
is the base of the Live Physical Time (LPT) structure shared with
the guest. This must be 64 bytes long.
The other is the base of an array of Stolen Time (ST) structures,
one for each VCPU. There must be (64 * total number of VCPUs) bytes
of memory available at this location.
The addresses are given in terms of the physical address visible to
the guest and must be 64 byte aligned. The memory should be marked as
reserved to the guest to stop it allocating it for other purposes.
A method is also provided to change the paravirtualized clock
frequency seen by a guest. This can only be changed before the guest
has been started, otherwise the guest will become confused.
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/include/asm/kvm_mmu.h | 2 +
arch/arm64/include/uapi/asm/kvm.h | 8 +
arch/arm64/kvm/Makefile | 1 +
include/uapi/linux/kvm.h | 2 +
virt/kvm/arm/mmu.c | 44 ++++++
virt/kvm/arm/pvtime.c | 243 ++++++++++++++++++++++++++++++
6 files changed, 300 insertions(+)
create mode 100644 virt/kvm/arm/pvtime.c
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 658657367f2f..3c266521223a 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -167,6 +167,8 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm);
void kvm_free_stage2_pgd(struct kvm *kvm);
int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
phys_addr_t pa, unsigned long size, bool writable);
+int kvm_phys_addr_memremap(struct kvm *kvm, phys_addr_t guest_ipa,
+ phys_addr_t pa, unsigned long size, bool writable);
int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run);
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 97c3478ee6e7..157bd8d7ec82 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -307,6 +307,14 @@ struct kvm_vcpu_events {
#define KVM_PSCI_RET_INVAL PSCI_RET_INVALID_PARAMS
#define KVM_PSCI_RET_DENIED PSCI_RET_DENIED
+/* Device Control API: PV_TIME */
+#define KVM_DEV_ARM_PV_TIME_PADDR 0
+#define KVM_DEV_ARM_PV_TIME_LPT 0
+#define KVM_DEV_ARM_PV_TIME_ST 1
+#define KVM_DEV_ARM_PV_TIME_FREQUENCY 1
+#define KVM_DEV_ARM_PV_TIME_STATE_SIZE 2
+#define KVM_DEV_ARM_PV_TIME_STATE 3
+
#endif
#endif /* __ARM_KVM_H__ */
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 45bb68f6c161..a3f064f0a51d 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -16,6 +16,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/e
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hypercalls.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/pvtime.o
kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 2b7a652c9fa4..494f7ad7987e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1198,6 +1198,8 @@ enum kvm_device_type {
#define KVM_DEV_TYPE_ARM_VGIC_V3 KVM_DEV_TYPE_ARM_VGIC_V3
KVM_DEV_TYPE_ARM_VGIC_ITS,
#define KVM_DEV_TYPE_ARM_VGIC_ITS KVM_DEV_TYPE_ARM_VGIC_ITS
+ KVM_DEV_TYPE_ARM_PV_TIME,
+#define KVM_DEV_TYPE_ARM_PV_TIME KVM_DEV_TYPE_ARM_PV_TIME
KVM_DEV_TYPE_MAX,
};
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 5eca48bdb1a6..17ec31c40039 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1227,6 +1227,50 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
return ret;
}
+/**
+ * kvm_phys_addr_memremap - map a memory range to guest IPA
+ *
+ * @kvm: The KVM pointer
+ * @guest_ipa: The IPA at which to insert the mapping
+ * @pa: The physical address of the memory
+ * @size: The size of the mapping
+ */
+int kvm_phys_addr_memremap(struct kvm *kvm, phys_addr_t guest_ipa,
+ phys_addr_t pa, unsigned long size, bool writable)
+{
+ phys_addr_t addr, end;
+ int ret = 0;
+ unsigned long pfn;
+ struct kvm_mmu_memory_cache cache = { 0, };
+
+ end = (guest_ipa + size + PAGE_SIZE - 1) & PAGE_MASK;
+ pfn = __phys_to_pfn(pa);
+
+ for (addr = guest_ipa; addr < end; addr += PAGE_SIZE) {
+ pte_t pte = pfn_pte(pfn, PAGE_S2);
+
+ if (writable)
+ pte = kvm_s2pte_mkwrite(pte);
+
+ ret = mmu_topup_memory_cache(&cache,
+ kvm_mmu_cache_min_pages(kvm),
+ KVM_NR_MEM_OBJS);
+ if (ret)
+ goto out;
+ spin_lock(&kvm->mmu_lock);
+ ret = stage2_set_pte(kvm, &cache, addr, &pte, 0);
+ spin_unlock(&kvm->mmu_lock);
+ if (ret)
+ goto out;
+
+ pfn++;
+ }
+
+out:
+ mmu_free_memory_cache(&cache);
+ return ret;
+}
+
static bool transparent_hugepage_adjust(kvm_pfn_t *pfnp, phys_addr_t *ipap)
{
kvm_pfn_t pfn = *pfnp;
diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
new file mode 100644
index 000000000000..16efa7e70424
--- /dev/null
+++ b/virt/kvm/arm/pvtime.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Arm Ltd.
+
+#include <linux/kvm_host.h>
+#include <asm/kvm_mmu.h>
+
+/* We currently only support PV time on ARM64 */
+#ifdef CONFIG_ARM64
+
+static int max_stolen_size(void)
+{
+ int size = KVM_MAX_VCPUS * sizeof(struct pvclock_vcpu_stolen_time_info);
+
+ return ALIGN(size, PAGE_SIZE);
+}
+
+static int kvm_arm_pvtime_create(struct kvm_device *dev, u32 type)
+{
+ int size_required;
+ struct kvm_arch_pvtime *pvtime = &dev->kvm->arch.pvtime;
+
+ /*
+ * We require 1 page for LPT and a pvclock_vcpu_stolen_time_info
+ * structure per VCPU.
+ */
+ size_required = PAGE_SIZE + max_stolen_size();
+
+ pvtime->lpt_fpv = arch_timer_get_rate();
+ pvtime->pv_page = alloc_pages_exact(size_required,
+ GFP_KERNEL | __GFP_ZERO);
+
+ if (!pvtime->pv_page)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kvm_arm_pvtime_destroy(struct kvm_device *dev)
+{
+ struct kvm_arch_pvtime *pvtime = &dev->kvm->arch.pvtime;
+
+ pvtime->lpt_page = GPA_INVALID;
+ pvtime->st_base = GPA_INVALID;
+ free_pages_exact(pvtime->pv_page, PAGE_SIZE + max_stolen_size());
+}
+
+static int pvtime_map_pages(struct kvm *kvm, gpa_t guest_paddr,
+ int offset, int size)
+{
+ struct kvm_arch_pvtime *pvtime = &kvm->arch.pvtime;
+
+ return kvm_phys_addr_memremap(kvm, guest_paddr,
+ virt_to_phys(pvtime->pv_page + offset),
+ size, false);
+}
+
+static int pvtime_save_state(struct kvm *kvm, u64 type, void __user *user)
+{
+ void *source;
+ size_t size;
+
+ switch (type) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ source = kvm->arch.pvtime.pv_page;
+ size = sizeof(struct pvclock_vm_time_info);
+ break;
+ case KVM_DEV_ARM_PV_TIME_ST:
+ source = kvm->arch.pvtime.pv_page + PAGE_SIZE;
+ size = sizeof(struct pvclock_vcpu_stolen_time_info) *
+ atomic_read(&kvm->online_vcpus);
+ break;
+ default:
+ return -ENXIO;
+ }
+
+ if (copy_to_user(user, source, size))
+ return -EFAULT;
+ return 0;
+}
+
+static int pvtime_restore_state(struct kvm *kvm, u64 type, void __user *user)
+{
+ void *dest;
+ size_t size;
+
+ switch (type) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ dest = kvm->arch.pvtime.pv_page;
+ size = sizeof(struct pvclock_vm_time_info);
+ break;
+ case KVM_DEV_ARM_PV_TIME_ST:
+ dest = kvm->arch.pvtime.pv_page + PAGE_SIZE;
+ size = sizeof(struct pvclock_vcpu_stolen_time_info) *
+ atomic_read(&kvm->online_vcpus);
+ break;
+ default:
+ return -ENXIO;
+ }
+
+ if (copy_from_user(dest, user, size))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int kvm_arm_pvtime_set_attr(struct kvm_device *dev,
+ struct kvm_device_attr *attr)
+{
+ struct kvm_arch_pvtime *pvtime = &dev->kvm->arch.pvtime;
+ u64 __user *user = (u64 __user *)attr->addr;
+ u64 paddr;
+ u32 frequency;
+ int ret;
+
+ switch (attr->group) {
+ case KVM_DEV_ARM_PV_TIME_PADDR:
+ if (get_user(paddr, user))
+ return -EFAULT;
+ if (paddr & 63)
+ return -EINVAL;
+ switch (attr->attr) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ if (pvtime->lpt_page != GPA_INVALID)
+ return -EEXIST;
+ ret = pvtime_map_pages(dev->kvm, paddr, 0, PAGE_SIZE);
+ if (ret)
+ return ret;
+ pvtime->lpt_page = paddr;
+ return kvm_arm_update_lpt_sequence(dev->kvm);
+ case KVM_DEV_ARM_PV_TIME_ST:
+ if (pvtime->st_base != GPA_INVALID)
+ return -EEXIST;
+ ret = pvtime_map_pages(dev->kvm, paddr, PAGE_SIZE,
+ max_stolen_size());
+ if (ret)
+ return ret;
+ pvtime->st_base = paddr;
+ return 0;
+ }
+ break;
+ case KVM_DEV_ARM_PV_TIME_FREQUENCY:
+ if (attr->attr != KVM_DEV_ARM_PV_TIME_LPT)
+ break;
+ if (get_user(frequency, user))
+ return -EFAULT;
+ pvtime->lpt_fpv = frequency;
+ return kvm_arm_update_lpt_sequence(dev->kvm);
+ case KVM_DEV_ARM_PV_TIME_STATE_SIZE:
+ return -EPERM;
+ case KVM_DEV_ARM_PV_TIME_STATE:
+ return pvtime_restore_state(dev->kvm, attr->attr, user);
+ }
+ return -ENXIO;
+}
+
+static int kvm_arm_pvtime_get_attr(struct kvm_device *dev,
+ struct kvm_device_attr *attr)
+{
+ u64 __user *user = (u64 __user *)attr->addr;
+ u32 size;
+
+ switch (attr->group) {
+ case KVM_DEV_ARM_PV_TIME_PADDR:
+ switch (attr->attr) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ if (put_user(dev->kvm->arch.pvtime.lpt_page, user))
+ return -EFAULT;
+ return 0;
+ case KVM_DEV_ARM_PV_TIME_ST:
+ if (put_user(dev->kvm->arch.pvtime.st_base, user))
+ return -EFAULT;
+ return 0;
+ }
+ break;
+ case KVM_DEV_ARM_PV_TIME_FREQUENCY:
+ if (attr->attr != KVM_DEV_ARM_PV_TIME_LPT)
+ break;
+ if (put_user(dev->kvm->arch.pvtime.lpt_fpv, user))
+ return -EFAULT;
+ return 0;
+ case KVM_DEV_ARM_PV_TIME_STATE_SIZE:
+ switch (attr->attr) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ size = sizeof(struct pvclock_vm_time_info);
+ break;
+ case KVM_DEV_ARM_PV_TIME_ST:
+ size = sizeof(struct pvclock_vcpu_stolen_time_info);
+ size *= atomic_read(&dev->kvm->online_vcpus);
+ break;
+ default:
+ return -ENXIO;
+ }
+ if (put_user(size, user))
+ return -EFAULT;
+ return 0;
+ case KVM_DEV_ARM_PV_TIME_STATE:
+ return pvtime_save_state(dev->kvm, attr->attr, user);
+ }
+ return -ENXIO;
+}
+
+static int kvm_arm_pvtime_has_attr(struct kvm_device *dev,
+ struct kvm_device_attr *attr)
+{
+ switch (attr->group) {
+ case KVM_DEV_ARM_PV_TIME_PADDR:
+ case KVM_DEV_ARM_PV_TIME_STATE_SIZE:
+ case KVM_DEV_ARM_PV_TIME_STATE:
+ switch (attr->attr) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ case KVM_DEV_ARM_PV_TIME_ST:
+ return 0;
+ }
+ break;
+ case KVM_DEV_ARM_PV_TIME_FREQUENCY:
+ switch (attr->attr) {
+ case KVM_DEV_ARM_PV_TIME_LPT:
+ return 0;
+ }
+ break;
+ }
+ return -ENXIO;
+}
+
+static const struct kvm_device_ops pvtime_ops = {
+ "Arm PV time",
+ .create = kvm_arm_pvtime_create,
+ .destroy = kvm_arm_pvtime_destroy,
+ .set_attr = kvm_arm_pvtime_set_attr,
+ .get_attr = kvm_arm_pvtime_get_attr,
+ .has_attr = kvm_arm_pvtime_has_attr
+};
+
+static int __init kvm_pvtime_init(void)
+{
+ kvm_register_device_ops(&pvtime_ops, KVM_DEV_TYPE_ARM_PV_TIME);
+
+ return 0;
+}
+
+late_initcall(kvm_pvtime_init);
+
+#endif
--
2.19.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 00/12] arm64: Paravirtualized time support
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (11 preceding siblings ...)
2018-11-28 14:45 ` [PATCH 12/12] KVM: arm64: Provide a PV_TIME device to user space Steven Price
@ 2018-12-03 13:25 ` Andrew Jones
2018-12-03 14:36 ` Marc Zyngier
2018-12-05 12:30 ` Steven Price
2018-12-10 11:40 ` Mark Rutland
13 siblings, 2 replies; 32+ messages in thread
From: Andrew Jones @ 2018-12-03 13:25 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:15PM +0000, Steven Price wrote:
> This series add support for paravirtualized time for Arm64 guests and
> KVM hosts following the specification in Arm's document DEN 0057A:
>
> https://developer.arm.com/docs/den0057/a
Hi Steven,
As that specification is still a draft, then I guess this series is an
RFC. I just wanted to point that out, as I believe that tag should be
used in future postings until the spec is approved.
Regarding the spec, my understanding from kvm forum was that there was
still a need to explain why kvmclock, or an extension to kvmclock, is
insufficient.
If there hasn't been anything written up about that yet, would you mind
doing so?
>
> It implements support for Live Physical Time (LPT) which provides the
> guest with a method to derive a stable counter of time during which the
> guest is executing even when the guest is being migrated between hosts
> with different physical counter frequencies.
Intel has TSC scaling. Is there any reason Arm is proposing a PV
solution instead of adding a similar virt extension?
Thanks,
drew
>
> It also implements support for stolen time, allowing the guest to
> identify time when it is forcibly not executing.
>
> Patch 1 provides some documentation
> Patches 2-4, 8 and 11 provide some refactoring of existing code
> Patch 5 implements the new PV_FEATURES discovery mechanism
> Patches 6-7 implement live physical time
> Patches 9-10 implement stolen time
> Patch 12 adds the 'PV_TIME' device for user space to enable the features
>
> Christoffer Dall (2):
> KVM: arm/arm64: Factor out hypercall handling from PSCI code
> KVM: Export mark_page_dirty_in_slot
>
> Steven Price (10):
> KVM: arm64: Document PV-time interface
> arm/arm64: Provide a wrapper for SMCCC 1.1 calls
> arm/arm64: Make use of the SMCCC 1.1 wrapper
> KVM: arm64: Implement PV_FEATURES call
> KVM: arm64: Support Live Physical Time reporting
> clocksource: arm_arch_timer: Use paravirtualized LPT
> KVM: arm64: Support stolen time reporting via shared page
> arm64: Retrieve stolen time as paravirtualized guest
> KVM: Allow kvm_device_ops to be const
> KVM: arm64: Provide a PV_TIME device to user space
>
> Documentation/virtual/kvm/arm/pvtime.txt | 169 ++++++++++++++
> arch/arm/kvm/Makefile | 2 +-
> arch/arm/kvm/handle_exit.c | 2 +-
> arch/arm/mm/proc-v7-bugs.c | 46 ++--
> arch/arm64/include/asm/arch_timer.h | 32 ++-
> arch/arm64/include/asm/kvm_host.h | 16 ++
> arch/arm64/include/asm/kvm_mmu.h | 2 +
> arch/arm64/include/asm/pvclock-abi.h | 32 +++
> arch/arm64/include/uapi/asm/kvm.h | 8 +
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/cpu_errata.c | 47 +---
> arch/arm64/kernel/cpuinfo.c | 2 +-
> arch/arm64/kernel/kvm.c | 156 +++++++++++++
> arch/arm64/kvm/Kconfig | 1 +
> arch/arm64/kvm/Makefile | 2 +
> arch/arm64/kvm/handle_exit.c | 4 +-
> drivers/clocksource/arm_arch_timer.c | 176 ++++++++++++++-
> include/kvm/arm_arch_timer.h | 2 +
> include/kvm/arm_hypercalls.h | 44 ++++
> include/kvm/arm_psci.h | 2 +-
> include/kvm/arm_pv.h | 28 +++
> include/linux/arm-smccc.h | 45 ++++
> include/linux/cpuhotplug.h | 1 +
> include/linux/kvm_host.h | 5 +-
> include/linux/kvm_types.h | 2 +
> include/uapi/linux/kvm.h | 2 +
> virt/kvm/arm/arm.c | 25 +-
> virt/kvm/arm/hypercalls.c | 276 +++++++++++++++++++++++
> virt/kvm/arm/mmu.c | 44 ++++
> virt/kvm/arm/psci.c | 76 +------
> virt/kvm/arm/pvtime.c | 243 ++++++++++++++++++++
> virt/kvm/kvm_main.c | 12 +-
> 32 files changed, 1348 insertions(+), 157 deletions(-)
> create mode 100644 Documentation/virtual/kvm/arm/pvtime.txt
> create mode 100644 arch/arm64/include/asm/pvclock-abi.h
> create mode 100644 arch/arm64/kernel/kvm.c
> create mode 100644 include/kvm/arm_hypercalls.h
> create mode 100644 include/kvm/arm_pv.h
> create mode 100644 virt/kvm/arm/hypercalls.c
> create mode 100644 virt/kvm/arm/pvtime.c
>
> --
> 2.19.2
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 00/12] arm64: Paravirtualized time support
2018-12-03 13:25 ` [PATCH 00/12] arm64: Paravirtualized time support Andrew Jones
@ 2018-12-03 14:36 ` Marc Zyngier
2018-12-05 12:30 ` Steven Price
1 sibling, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2018-12-03 14:36 UTC (permalink / raw)
To: Andrew Jones, Steven Price
Cc: Catalin Marinas, Will Deacon, kvmarm, linux-arm-kernel
On 03/12/2018 13:25, Andrew Jones wrote:
> On Wed, Nov 28, 2018 at 02:45:15PM +0000, Steven Price wrote:
[...]
>> It implements support for Live Physical Time (LPT) which provides the
>> guest with a method to derive a stable counter of time during which the
>> guest is executing even when the guest is being migrated between hosts
>> with different physical counter frequencies.
>
> Intel has TSC scaling. Is there any reason Arm is proposing a PV
> solution instead of adding a similar virt extension?
Even if we were to add such an extension tomorrow, we need something
that works for today's system. Not to mention that "tomorrow in the
architecture" usually translates as "in silicon in 3 years"...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 00/12] arm64: Paravirtualized time support
2018-12-03 13:25 ` [PATCH 00/12] arm64: Paravirtualized time support Andrew Jones
2018-12-03 14:36 ` Marc Zyngier
@ 2018-12-05 12:30 ` Steven Price
1 sibling, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-12-05 12:30 UTC (permalink / raw)
To: Andrew Jones
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On 03/12/2018 13:25, Andrew Jones wrote:
> On Wed, Nov 28, 2018 at 02:45:15PM +0000, Steven Price wrote:
>> This series add support for paravirtualized time for Arm64 guests and
>> KVM hosts following the specification in Arm's document DEN 0057A:
>>
>> https://developer.arm.com/docs/den0057/a
>
> Hi Steven,
>
> As that specification is still a draft, then I guess this series is an
> RFC. I just wanted to point that out, as I believe that tag should be
> used in future postings until the spec is approved.
Hi,
Yes, sorry I should have included the RFC tag as we want feedback before
the specification is finalised.
> Regarding the spec, my understanding from kvm forum was that there was
> still a need to explain why kvmclock, or an extension to kvmclock, is
> insufficient.
>
> If there hasn't been anything written up about that yet, would you mind
> doing so?
There are obviously similarities to kvmclock, but there are a few
differences that enable some optimisations:
* The coefficient for scaling from native frequency to PV frequency is
a 64 bit integer that can be efficiently multiplied on arm64 hardware.
kvmclock provides us with a 32 bit multiplier.
* Rather than providing an offset in the structure (via
tsc_timestamp/system_time) the CPU's CNTVOFF support is used to provide
a virtual counter than simply needs scaling (no offset is applied by the
guest).
Given this is seemed sensible not to confuse matters by (ab)using the
existing kvmclock implementation.
>>
>> It implements support for Live Physical Time (LPT) which provides the
>> guest with a method to derive a stable counter of time during which the
>> guest is executing even when the guest is being migrated between hosts
>> with different physical counter frequencies.
>
> Intel has TSC scaling. Is there any reason Arm is proposing a PV
> solution instead of adding a similar virt extension?
As Marc has already pointed out - hardware changes will take a while to
happen, this works for today's systems.
Thanks,
Steve
>
> Thanks,
> drew
>
>>
>> It also implements support for stolen time, allowing the guest to
>> identify time when it is forcibly not executing.
>>
>> Patch 1 provides some documentation
>> Patches 2-4, 8 and 11 provide some refactoring of existing code
>> Patch 5 implements the new PV_FEATURES discovery mechanism
>> Patches 6-7 implement live physical time
>> Patches 9-10 implement stolen time
>> Patch 12 adds the 'PV_TIME' device for user space to enable the features
>>
>> Christoffer Dall (2):
>> KVM: arm/arm64: Factor out hypercall handling from PSCI code
>> KVM: Export mark_page_dirty_in_slot
>>
>> Steven Price (10):
>> KVM: arm64: Document PV-time interface
>> arm/arm64: Provide a wrapper for SMCCC 1.1 calls
>> arm/arm64: Make use of the SMCCC 1.1 wrapper
>> KVM: arm64: Implement PV_FEATURES call
>> KVM: arm64: Support Live Physical Time reporting
>> clocksource: arm_arch_timer: Use paravirtualized LPT
>> KVM: arm64: Support stolen time reporting via shared page
>> arm64: Retrieve stolen time as paravirtualized guest
>> KVM: Allow kvm_device_ops to be const
>> KVM: arm64: Provide a PV_TIME device to user space
>>
>> Documentation/virtual/kvm/arm/pvtime.txt | 169 ++++++++++++++
>> arch/arm/kvm/Makefile | 2 +-
>> arch/arm/kvm/handle_exit.c | 2 +-
>> arch/arm/mm/proc-v7-bugs.c | 46 ++--
>> arch/arm64/include/asm/arch_timer.h | 32 ++-
>> arch/arm64/include/asm/kvm_host.h | 16 ++
>> arch/arm64/include/asm/kvm_mmu.h | 2 +
>> arch/arm64/include/asm/pvclock-abi.h | 32 +++
>> arch/arm64/include/uapi/asm/kvm.h | 8 +
>> arch/arm64/kernel/Makefile | 1 +
>> arch/arm64/kernel/cpu_errata.c | 47 +---
>> arch/arm64/kernel/cpuinfo.c | 2 +-
>> arch/arm64/kernel/kvm.c | 156 +++++++++++++
>> arch/arm64/kvm/Kconfig | 1 +
>> arch/arm64/kvm/Makefile | 2 +
>> arch/arm64/kvm/handle_exit.c | 4 +-
>> drivers/clocksource/arm_arch_timer.c | 176 ++++++++++++++-
>> include/kvm/arm_arch_timer.h | 2 +
>> include/kvm/arm_hypercalls.h | 44 ++++
>> include/kvm/arm_psci.h | 2 +-
>> include/kvm/arm_pv.h | 28 +++
>> include/linux/arm-smccc.h | 45 ++++
>> include/linux/cpuhotplug.h | 1 +
>> include/linux/kvm_host.h | 5 +-
>> include/linux/kvm_types.h | 2 +
>> include/uapi/linux/kvm.h | 2 +
>> virt/kvm/arm/arm.c | 25 +-
>> virt/kvm/arm/hypercalls.c | 276 +++++++++++++++++++++++
>> virt/kvm/arm/mmu.c | 44 ++++
>> virt/kvm/arm/psci.c | 76 +------
>> virt/kvm/arm/pvtime.c | 243 ++++++++++++++++++++
>> virt/kvm/kvm_main.c | 12 +-
>> 32 files changed, 1348 insertions(+), 157 deletions(-)
>> create mode 100644 Documentation/virtual/kvm/arm/pvtime.txt
>> create mode 100644 arch/arm64/include/asm/pvclock-abi.h
>> create mode 100644 arch/arm64/kernel/kvm.c
>> create mode 100644 include/kvm/arm_hypercalls.h
>> create mode 100644 include/kvm/arm_pv.h
>> create mode 100644 virt/kvm/arm/hypercalls.c
>> create mode 100644 virt/kvm/arm/pvtime.c
>>
>> --
>> 2.19.2
>>
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm@lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 00/12] arm64: Paravirtualized time support
2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price
` (12 preceding siblings ...)
2018-12-03 13:25 ` [PATCH 00/12] arm64: Paravirtualized time support Andrew Jones
@ 2018-12-10 11:40 ` Mark Rutland
2018-12-10 16:08 ` Steven Price
2019-01-08 10:36 ` Christoffer Dall
13 siblings, 2 replies; 32+ messages in thread
From: Mark Rutland @ 2018-12-10 11:40 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
On Wed, Nov 28, 2018 at 02:45:15PM +0000, Steven Price wrote:
> This series add support for paravirtualized time for Arm64 guests and
> KVM hosts following the specification in Arm's document DEN 0057A:
>
> https://developer.arm.com/docs/den0057/a
>
> It implements support for Live Physical Time (LPT) which provides the
> guest with a method to derive a stable counter of time during which the
> guest is executing even when the guest is being migrated between hosts
> with different physical counter frequencies.
>
> It also implements support for stolen time, allowing the guest to
> identify time when it is forcibly not executing.
I know that stolen time reporting is important, and I think that we
definitely want to pick up that part of the spec (once it is published
in some non-draft form).
However, I am very concerned with the pv-freq part of LPT, and I'd like
to avoid that if at all possible. I say that because:
* By design, it breaks architectural guarantees from the PoV of SW in
the guest.
A VM may host multiple SW agents serially (e.g. when booting, or
across kexec), or concurrently (e.g. Linux w/ EFI runtime services),
and the host has no way to tell whether all software in the guest will
function correctly. Due to this, it's not possible to have a guest
opt-in to the architecturally-broken timekeeping.
Existing guests will not work correctly once pv-freq is in use, and if
configured without pv-freq (or if the guest fails to discover pv-freq
for any reason), the administrator may encounter anything between
subtle breakage or fatally incorrect timekeeping.
There's plenty of SW agents other than Linux which runs in a guest,
which would need to be updated to handle pv-freq, e.g. GRUB, *BSD,
iPXE.
Given this, I think that this is going to lead to subtle breakage in
real-world scenarios.
* It is (necessarily) invasive to the low-level arch timer code. This is
unfortunate, and I strongly suspect this is going to be an area with
long-term subtle breakage.
* It's not clear to me how strongly people need this. My understanding
is that datacenters would run largely homogeneous platforms. I suspect
large datacenters which would use migration are in a position to
mandate a standard timer frequency from their OEMs or SiPs.
I strongly believe that an architectural fix (e.g. in-hw scaling)
would be the better solution.
I understand that LPT is supposed to account for time lost during the
migration. Can we account for this without pv-freq? e.g. is it possible
to account for this in the same way as stolen time?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 00/12] arm64: Paravirtualized time support
2018-12-10 11:40 ` Mark Rutland
@ 2018-12-10 16:08 ` Steven Price
2019-01-08 10:36 ` Christoffer Dall
1 sibling, 0 replies; 32+ messages in thread
From: Steven Price @ 2018-12-10 16:08 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, kvmarm,
linux-arm-kernel
On 10/12/2018 11:40, Mark Rutland wrote:
> On Wed, Nov 28, 2018 at 02:45:15PM +0000, Steven Price wrote:
>> This series add support for paravirtualized time for Arm64 guests and
>> KVM hosts following the specification in Arm's document DEN 0057A:
>>
>> https://developer.arm.com/docs/den0057/a
>>
>> It implements support for Live Physical Time (LPT) which provides the
>> guest with a method to derive a stable counter of time during which the
>> guest is executing even when the guest is being migrated between hosts
>> with different physical counter frequencies.
>>
>> It also implements support for stolen time, allowing the guest to
>> identify time when it is forcibly not executing.
>
> I know that stolen time reporting is important, and I think that we
> definitely want to pick up that part of the spec (once it is published
> in some non-draft form).
>
> However, I am very concerned with the pv-freq part of LPT, and I'd like
> to avoid that if at all possible. I say that because:
>
> * By design, it breaks architectural guarantees from the PoV of SW in
> the guest.
>
> A VM may host multiple SW agents serially (e.g. when booting, or
> across kexec), or concurrently (e.g. Linux w/ EFI runtime services),
> and the host has no way to tell whether all software in the guest will
> function correctly. Due to this, it's not possible to have a guest
> opt-in to the architecturally-broken timekeeping.
>
> Existing guests will not work correctly once pv-freq is in use, and if
> configured without pv-freq (or if the guest fails to discover pv-freq
> for any reason), the administrator may encounter anything between
> subtle breakage or fatally incorrect timekeeping.
>
> There's plenty of SW agents other than Linux which runs in a guest,
> which would need to be updated to handle pv-freq, e.g. GRUB, *BSD,
> iPXE.
>
> Given this, I think that this is going to lead to subtle breakage in
> real-world scenarios.
LPT only changes things on migration. Up until migration the
(architectural) clocks still behave perfectly normally. A guest which
opts in to LPT can derive a clock with a different frequency, but the
underlying clock doesn't change.
When migration happens it's a different story.
If the frequency of the new host matches the old host then again the
clocks behave 'normally': CNTVOFF is used to hide the change in offset
such that the guest at worst sees time pause during the actual migration.
But the whole point of LPT is to deal with the situation if the clock
frequency has changed. A guest (or SW agent) which doesn't know about PV
will experience one of two things:
* Without LPT: the clock frequency will suddenly change without warning,
but the virtual counter is monotonically increasing.
* With LPT: the clock frequency will suddenly change and the virtual
counter will jump (it won't be monotonically increasing).
So I agree the situation with LPT is worse (we lose the monotonicity),
but any guest/agent which didn't understand about the migration is in
trouble if it cares about time.
> * It is (necessarily) invasive to the low-level arch timer code. This is
> unfortunate, and I strongly suspect this is going to be an area with
> long-term subtle breakage.
I can't argue against that - I've tried to limit how invasive the code
changes are, but ultimately we're changing the interpretation of
low-level timers.
> * It's not clear to me how strongly people need this. My understanding
> is that datacenters would run largely homogeneous platforms. I suspect
> large datacenters which would use migration are in a position to
> mandate a standard timer frequency from their OEMs or SiPs.
>
> I strongly believe that an architectural fix (e.g. in-hw scaling)
> would be the better solution.
An architectural fix in hardware is clearly the best solution. The
question is whether we want to support the use-case with today's
hardware. While mandating a particular 'standard' timer frequency is a
good idea, there's currently no standard. Large datacenters might be
able to mandate that, and maybe there'll be sufficient consensus that
this doesn't matter. But I seem to have misplaced my crystal ball...
> I understand that LPT is supposed to account for time lost during the
> migration. Can we account for this without pv-freq? e.g. is it possible
> to account for this in the same way as stolen time?
LPT isn't really about accounting for the time lost (to some extent this
is already done by saving/restoring the "KVM_REG_ARM_TIMER_CNT"
register) but about ensuring that the guest can derive a monotonically
increasing counter which maintains a stable frequency when migrated.
I'm going to respin the series with the LPT parts split out to the end,
that way we can (hopefully) agree on the stolen time parts and can defer
the LPT part if necessary.
Thanks,
Steve
> Thanks,
> Mark.
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 00/12] arm64: Paravirtualized time support
2018-12-10 11:40 ` Mark Rutland
2018-12-10 16:08 ` Steven Price
@ 2019-01-08 10:36 ` Christoffer Dall
1 sibling, 0 replies; 32+ messages in thread
From: Christoffer Dall @ 2019-01-08 10:36 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Steven Price, kvmarm,
linux-arm-kernel
On Mon, Dec 10, 2018 at 11:40:47AM +0000, Mark Rutland wrote:
> On Wed, Nov 28, 2018 at 02:45:15PM +0000, Steven Price wrote:
> > This series add support for paravirtualized time for Arm64 guests and
> > KVM hosts following the specification in Arm's document DEN 0057A:
> >
> > https://developer.arm.com/docs/den0057/a
> >
> > It implements support for Live Physical Time (LPT) which provides the
> > guest with a method to derive a stable counter of time during which the
> > guest is executing even when the guest is being migrated between hosts
> > with different physical counter frequencies.
> >
> > It also implements support for stolen time, allowing the guest to
> > identify time when it is forcibly not executing.
>
> I know that stolen time reporting is important, and I think that we
> definitely want to pick up that part of the spec (once it is published
> in some non-draft form).
>
> However, I am very concerned with the pv-freq part of LPT, and I'd like
> to avoid that if at all possible. I say that because:
>
> * By design, it breaks architectural guarantees from the PoV of SW in
> the guest.
>
> A VM may host multiple SW agents serially (e.g. when booting, or
> across kexec), or concurrently (e.g. Linux w/ EFI runtime services),
> and the host has no way to tell whether all software in the guest will
> function correctly. Due to this, it's not possible to have a guest
> opt-in to the architecturally-broken timekeeping.
Is this necessarily true?
As I understood the intention of the spec, there would be no change to
behavior of the timers as exposed by the hypervisor unless a software
agent specifically ops-int to LPT and pv-freq.
In a scenario with Linux and UEFI running, they must clearly agree on
using functionality that changes the underlying behavior. For
kdump/kexec scenarios, the OS would have to tear down the functionality
to work across migration after loading a secondary SW agent, which
probably needs adding to the spec.
>
> Existing guests will not work correctly once pv-freq is in use, and if
> configured without pv-freq (or if the guest fails to discover pv-freq
> for any reason), the administrator may encounter anything between
> subtle breakage or fatally incorrect timekeeping.
>
> There's plenty of SW agents other than Linux which runs in a guest,
> which would need to be updated to handle pv-freq, e.g. GRUB, *BSD,
> iPXE.
>
> Given this, I think that this is going to lead to subtle breakage in
> real-world scenarios.
I think we'd definitely need to limit the exposure of pv-freq to Linux
and (if necessary) UEFI runtime services. Do you see scenarios where
this would not be possible?
[...]
>
> I understand that LPT is supposed to account for time lost during the
> migration. Can we account for this without pv-freq? e.g. is it possible
> to account for this in the same way as stolen time?
>
I think we can indeed account for lost time during migration or host
system suspend by simply adjusting CNTVOFF_EL2 (as Steve points out, KVM
already supports this, but QEMU doesn't make use of that today -- there
were some patches attempting to address that recently).
Thanks,
Christoffer
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 32+ messages in thread