Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300852 > unrolled thread

Re: [PATCH RFC 2/3] x86/xen/time: setup vcpu 0 time info page

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH RFC 2/3] x86/xen/time: setup vcpu 0 time info page Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-01-04 17:10 +0100
    Re: [PATCH RFC 2/3] x86/xen/time: setup vcpu 0 time info page Joao Martins <joao.m.martins@oracle.com> - 2016-01-04 21:50 +0100
      Re: [PATCH RFC 2/3] x86/xen/time: setup vcpu 0 time info page Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-01-04 22:40 +0100
        Re: [PATCH RFC 2/3] x86/xen/time: setup vcpu 0 time info page Joao Martins <joao.m.martins@oracle.com> - 2016-01-05 13:30 +0100

#1300852 — Re: [PATCH RFC 2/3] x86/xen/time: setup vcpu 0 time info page

On 12/28/2015 04:52 PM, Joao Martins wrote:
> +
> +static int xen_setup_vsyscall_time_info(int cpu)
> +{
> +	struct pvclock_vsyscall_time_info *ti;
> +	struct vcpu_register_time_memory_area t;
> +	struct pvclock_vcpu_time_info *pvti;
> +	unsigned long mem;
> +	int ret, size;
> +	u8 flags;
> +
> +	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area,
> +				 cpu, NULL);
> +	if (ret == -ENOSYS) {
> +		pr_debug("xen: vcpu_time_info placement not supported\n");
> +		return -ENOTSUPP;
> +	}

I don't think this is necessary.

> +
> +	size = PAGE_ALIGN(sizeof(struct pvclock_vsyscall_time_info));
> +	mem = memblock_alloc(size, PAGE_SIZE);
> +	if (!mem)
> +		return -ENOMEM;
> +
> +	ti = __va(mem);
> +	memset(ti, 0, size);

Can you just use get_zeroed_page()? (struct pvclock_vsyscall_time_info 
is always less than a page, isn't it?).


-boris

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1301076

On 01/04/2016 04:07 PM, Boris Ostrovsky wrote:
> On 12/28/2015 04:52 PM, Joao Martins wrote:
>> +
>> +static int xen_setup_vsyscall_time_info(int cpu)
>> +{
>> +	struct pvclock_vsyscall_time_info *ti;
>> +	struct vcpu_register_time_memory_area t;
>> +	struct pvclock_vcpu_time_info *pvti;
>> +	unsigned long mem;
>> +	int ret, size;
>> +	u8 flags;
>> +
>> +	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area,
>> +				 cpu, NULL);
>> +	if (ret == -ENOSYS) {
>> +		pr_debug("xen: vcpu_time_info placement not supported\n");
>> +		return -ENOTSUPP;
>> +	}
> 
> I don't think this is necessary.
> 
OK, I will remove it.

>> +
>> +	size = PAGE_ALIGN(sizeof(struct pvclock_vsyscall_time_info));
>> +	mem = memblock_alloc(size, PAGE_SIZE);
>> +	if (!mem)
>> +		return -ENOMEM;
>> +
>> +	ti = __va(mem);
>> +	memset(ti, 0, size);
> 
> Can you just use get_zeroed_page()? (struct pvclock_vsyscall_time_info 
> is always less than a page, isn't it?).
Yeah, I can use get_zeroed_page() (struct pvclock_vsyscall_time_info is always
less than a page).

Additionally perhaps this region shouldn't be freed if PVCLOCK_TSC_STABLE_BIT
isn't supported, because otherwise I would end up corrupting data elsewhere
since the pvti would still be periodically updated by Xen, right?

> 
> 
> -boris
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301113

On 01/04/2016 03:41 PM, Joao Martins wrote:
>
> On 01/04/2016 04:07 PM, Boris Ostrovsky wrote:
>> On 12/28/2015 04:52 PM, Joao Martins wrote:
>>
>>> +
>>> +	size = PAGE_ALIGN(sizeof(struct pvclock_vsyscall_time_info));
>>> +	mem = memblock_alloc(size, PAGE_SIZE);
>>> +	if (!mem)
>>> +		return -ENOMEM;
>>> +
>>> +	ti = __va(mem);
>>> +	memset(ti, 0, size);
>> Can you just use get_zeroed_page()? (struct pvclock_vsyscall_time_info
>> is always less than a page, isn't it?).
> Yeah, I can use get_zeroed_page() (struct pvclock_vsyscall_time_info is always
> less than a page).
>
> Additionally perhaps this region shouldn't be freed if PVCLOCK_TSC_STABLE_BIT
> isn't supported, because otherwise I would end up corrupting data elsewhere
> since the pvti would still be periodically updated by Xen, right?

You could try setting it back to NULL. e.g.
     if (!HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, cpu, 
NULL))
         free_page(..);

-boris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301487

On 01/04/2016 09:34 PM, Boris Ostrovsky wrote:
> On 01/04/2016 03:41 PM, Joao Martins wrote:
>>
>> On 01/04/2016 04:07 PM, Boris Ostrovsky wrote:
>>> On 12/28/2015 04:52 PM, Joao Martins wrote:
>>>
>>>> +
>>>> +	size = PAGE_ALIGN(sizeof(struct pvclock_vsyscall_time_info));
>>>> +	mem = memblock_alloc(size, PAGE_SIZE);
>>>> +	if (!mem)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	ti = __va(mem);
>>>> +	memset(ti, 0, size);
>>> Can you just use get_zeroed_page()? (struct pvclock_vsyscall_time_info
>>> is always less than a page, isn't it?).
>> Yeah, I can use get_zeroed_page() (struct pvclock_vsyscall_time_info is always
>> less than a page).
>>
>> Additionally perhaps this region shouldn't be freed if PVCLOCK_TSC_STABLE_BIT
>> isn't supported, because otherwise I would end up corrupting data elsewhere
>> since the pvti would still be periodically updated by Xen, right?
> 
> You could try setting it back to NULL. e.g.
>      if (!HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, cpu, 
> NULL))
>          free_page(..);
> 
Ah, forgot about that option. Thanks! We can't register it explicitly as NULL as
the hypercall would return -EFAULT but having a valid argument with NULL as the
address works nicely.

> -boris
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web