* [PATCH] replace bogus -ENOSYS uses
@ 2016-08-09 10:40 Jan Beulich
2016-08-11 18:10 ` Andrew Cooper
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Jan Beulich @ 2016-08-09 10:40 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 5078 bytes --]
This doesn't cover all of them, just the ones that I think would most
obviously better be -EINVAL or -EOPNOTSUPP.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -440,7 +440,7 @@ int unmmap_broken_page(struct domain *d,
return -EINVAL;
if ( !has_hvm_container_domain(d) || !paging_mode_hap(d) )
- return -ENOSYS;
+ return -EOPNOTSUPP;
rc = -1;
r_mfn = get_gfn_query(d, gfn, &pt);
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
printk(KERN_WARNING
"mtrr: your processor doesn't support write-combining\n");
- return -ENOSYS;
+ return -EOPNOTSUPP;
}
if (!size) {
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5592,7 +5592,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
break;
case HVMOP_flush_tlbs:
- rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
+ rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -EINVAL;
break;
case HVMOP_track_dirty_vram:
@@ -5832,7 +5832,7 @@ int hvm_debug_op(struct vcpu *v, int32_t
{
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
- rc = -ENOSYS;
+ rc = -EOPNOTSUPP;
if ( !cpu_has_monitor_trap_flag )
break;
rc = 0;
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3565,7 +3565,7 @@ long do_mmuext_op(
if ( !opt_allow_superpage )
{
MEM_LOG("Superpages disallowed");
- rc = -ENOSYS;
+ rc = -EOPNOTSUPP;
}
else if ( unlikely(d != pg_owner) )
rc = -EPERM;
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -353,7 +353,7 @@ int compat_memory_op(unsigned int cmd, X
struct get_reserved_device_memory grdm;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&grdm.map, compat, 1) ||
!compat_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -621,7 +621,7 @@ int evtchn_fifo_expand_array(const struc
int rc;
if ( !d->evtchn_fifo )
- return -ENOSYS;
+ return -EOPNOTSUPP;
spin_lock(&d->event_lock);
rc = add_page_to_event_array(d, expand_array->array_gfn);
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3025,7 +3025,7 @@ do_grant_table_op(
return -EINVAL;
if ( (cmd &= GNTTABOP_CMD_MASK) != GNTTABOP_cache_flush && opaque_in )
- return -ENOSYS;
+ return -EINVAL;
rc = -EFAULT;
switch ( cmd )
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -937,14 +937,14 @@ long do_memory_op(unsigned long cmd, XEN
case XENMEM_exchange:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
rc = memory_exchange(guest_handle_cast(arg, xen_memory_exchange_t));
break;
case XENMEM_maximum_ram_page:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
rc = max_page;
break;
@@ -953,7 +953,7 @@ long do_memory_op(unsigned long cmd, XEN
case XENMEM_maximum_reservation:
case XENMEM_maximum_gpfn:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&domid, arg, 1) )
return -EFAULT;
@@ -1077,7 +1077,7 @@ long do_memory_op(unsigned long cmd, XEN
struct page_info *page;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&xrfp, arg, 1) )
return -EFAULT;
@@ -1114,7 +1114,7 @@ long do_memory_op(unsigned long cmd, XEN
case XENMEM_claim_pages:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&reservation, arg, 1) )
return -EFAULT;
@@ -1148,7 +1148,7 @@ long do_memory_op(unsigned long cmd, XEN
struct vnuma_info tmp;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
/*
* Guest passes nr_vnodes, number of regions and nr_vcpus thus
@@ -1280,7 +1280,7 @@ long do_memory_op(unsigned long cmd, XEN
struct get_reserved_device_memory grdm;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&grdm.map, arg, 1) ||
!guest_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
[-- Attachment #2: replace-bogus-ENOSYS.patch --]
[-- Type: text/plain, Size: 5104 bytes --]
replace bogus -ENOSYS uses
This doesn't cover all of them, just the ones that I think would most
obviously better be -EINVAL or -EOPNOTSUPP.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -440,7 +440,7 @@ int unmmap_broken_page(struct domain *d,
return -EINVAL;
if ( !has_hvm_container_domain(d) || !paging_mode_hap(d) )
- return -ENOSYS;
+ return -EOPNOTSUPP;
rc = -1;
r_mfn = get_gfn_query(d, gfn, &pt);
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
printk(KERN_WARNING
"mtrr: your processor doesn't support write-combining\n");
- return -ENOSYS;
+ return -EOPNOTSUPP;
}
if (!size) {
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5592,7 +5592,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
break;
case HVMOP_flush_tlbs:
- rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
+ rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -EINVAL;
break;
case HVMOP_track_dirty_vram:
@@ -5832,7 +5832,7 @@ int hvm_debug_op(struct vcpu *v, int32_t
{
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
- rc = -ENOSYS;
+ rc = -EOPNOTSUPP;
if ( !cpu_has_monitor_trap_flag )
break;
rc = 0;
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3565,7 +3565,7 @@ long do_mmuext_op(
if ( !opt_allow_superpage )
{
MEM_LOG("Superpages disallowed");
- rc = -ENOSYS;
+ rc = -EOPNOTSUPP;
}
else if ( unlikely(d != pg_owner) )
rc = -EPERM;
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -353,7 +353,7 @@ int compat_memory_op(unsigned int cmd, X
struct get_reserved_device_memory grdm;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&grdm.map, compat, 1) ||
!compat_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -621,7 +621,7 @@ int evtchn_fifo_expand_array(const struc
int rc;
if ( !d->evtchn_fifo )
- return -ENOSYS;
+ return -EOPNOTSUPP;
spin_lock(&d->event_lock);
rc = add_page_to_event_array(d, expand_array->array_gfn);
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3025,7 +3025,7 @@ do_grant_table_op(
return -EINVAL;
if ( (cmd &= GNTTABOP_CMD_MASK) != GNTTABOP_cache_flush && opaque_in )
- return -ENOSYS;
+ return -EINVAL;
rc = -EFAULT;
switch ( cmd )
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -937,14 +937,14 @@ long do_memory_op(unsigned long cmd, XEN
case XENMEM_exchange:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
rc = memory_exchange(guest_handle_cast(arg, xen_memory_exchange_t));
break;
case XENMEM_maximum_ram_page:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
rc = max_page;
break;
@@ -953,7 +953,7 @@ long do_memory_op(unsigned long cmd, XEN
case XENMEM_maximum_reservation:
case XENMEM_maximum_gpfn:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&domid, arg, 1) )
return -EFAULT;
@@ -1077,7 +1077,7 @@ long do_memory_op(unsigned long cmd, XEN
struct page_info *page;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&xrfp, arg, 1) )
return -EFAULT;
@@ -1114,7 +1114,7 @@ long do_memory_op(unsigned long cmd, XEN
case XENMEM_claim_pages:
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&reservation, arg, 1) )
return -EFAULT;
@@ -1148,7 +1148,7 @@ long do_memory_op(unsigned long cmd, XEN
struct vnuma_info tmp;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
/*
* Guest passes nr_vnodes, number of regions and nr_vcpus thus
@@ -1280,7 +1280,7 @@ long do_memory_op(unsigned long cmd, XEN
struct get_reserved_device_memory grdm;
if ( unlikely(start_extent) )
- return -ENOSYS;
+ return -EINVAL;
if ( copy_from_guest(&grdm.map, arg, 1) ||
!guest_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-09 10:40 [PATCH] replace bogus -ENOSYS uses Jan Beulich
@ 2016-08-11 18:10 ` Andrew Cooper
2016-08-12 10:34 ` George Dunlap
2016-08-12 10:58 ` Jan Beulich
2016-08-29 14:01 ` Ping: " Jan Beulich
2016-09-05 13:20 ` George Dunlap
2 siblings, 2 replies; 11+ messages in thread
From: Andrew Cooper @ 2016-08-11 18:10 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson
On 09/08/16 11:40, Jan Beulich wrote:
> --- a/xen/arch/x86/cpu/mtrr/main.c
> +++ b/xen/arch/x86/cpu/mtrr/main.c
> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
> printk(KERN_WARNING
> "mtrr: your processor doesn't support write-combining\n");
> - return -ENOSYS;
> + return -EOPNOTSUPP;
Will this break the classic-xen MTRR code? ISTR it was very picky, from
the cpuid work. Also, as some further cleanup, that printk should
become a print-once.
The others look ok.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-11 18:10 ` Andrew Cooper
@ 2016-08-12 10:34 ` George Dunlap
2016-08-12 11:02 ` Jan Beulich
2016-08-12 10:58 ` Jan Beulich
1 sibling, 1 reply; 11+ messages in thread
From: George Dunlap @ 2016-08-12 10:34 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson
On 11/08/16 19:10, Andrew Cooper wrote:
> On 09/08/16 11:40, Jan Beulich wrote:
>> --- a/xen/arch/x86/cpu/mtrr/main.c
>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>> printk(KERN_WARNING
>> "mtrr: your processor doesn't support write-combining\n");
>> - return -ENOSYS;
>> + return -EOPNOTSUPP;
>
> Will this break the classic-xen MTRR code? ISTR it was very picky, from
> the cpuid work. Also, as some further cleanup, that printk should
> become a print-once.
>
> The others look ok.
That does bring up a good general point though -- the return value is
part of the ABI. Are you reasonably confident that none of the callers
will be confused when this return value changes? If so, a note in the
commit message justifying this confidence would be helpful I think.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-11 18:10 ` Andrew Cooper
2016-08-12 10:34 ` George Dunlap
@ 2016-08-12 10:58 ` Jan Beulich
2016-08-12 11:49 ` Andrew Cooper
1 sibling, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2016-08-12 10:58 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, xen-devel
>>> On 11.08.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
> On 09/08/16 11:40, Jan Beulich wrote:
>> --- a/xen/arch/x86/cpu/mtrr/main.c
>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>> printk(KERN_WARNING
>> "mtrr: your processor doesn't support write-combining\n");
>> - return -ENOSYS;
>> + return -EOPNOTSUPP;
>
> Will this break the classic-xen MTRR code? ISTR it was very picky, from
> the cpuid work.
There are no -ENOSYS checks in there afaics, and I also can't
otherwise see any way for this change to break it.
> Also, as some further cleanup, that printk should
> become a print-once.
Well, for a message that presumably would never actually get
issued (as I'm unaware of 64-bit capable CPUs not supporting
WC) I don't think this sort of cleanup has a really high priority.
Certainly not in this patch.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-12 10:34 ` George Dunlap
@ 2016-08-12 11:02 ` Jan Beulich
2016-08-17 13:40 ` Ping: " Jan Beulich
0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2016-08-12 11:02 UTC (permalink / raw)
To: George Dunlap
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
>>> On 12.08.16 at 12:34, <george.dunlap@citrix.com> wrote:
> On 11/08/16 19:10, Andrew Cooper wrote:
>> On 09/08/16 11:40, Jan Beulich wrote:
>>> --- a/xen/arch/x86/cpu/mtrr/main.c
>>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>>> printk(KERN_WARNING
>>> "mtrr: your processor doesn't support write-combining\n");
>>> - return -ENOSYS;
>>> + return -EOPNOTSUPP;
>>
>> Will this break the classic-xen MTRR code? ISTR it was very picky, from
>> the cpuid work. Also, as some further cleanup, that printk should
>> become a print-once.
>>
>> The others look ok.
>
> That does bring up a good general point though -- the return value is
> part of the ABI. Are you reasonably confident that none of the callers
> will be confused when this return value changes? If so, a note in the
> commit message justifying this confidence would be helpful I think.
I don't think specific return values can be considered part of the
ABI, or else we couldn't e.g. change the order in which certain
checks get performed.
And then please also consider a hypothetical future hypervisor with
the MTRR operations simply ripped out - that would return -ENOSYS
or -EOPNOTSUPP then too, without a way for the caller to tell that
more generic error condition from the more specific one here.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-12 10:58 ` Jan Beulich
@ 2016-08-12 11:49 ` Andrew Cooper
2016-09-06 8:03 ` Jan Beulich
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Cooper @ 2016-08-12 11:49 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, xen-devel
On 12/08/16 11:58, Jan Beulich wrote:
>>>> On 11.08.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
>> On 09/08/16 11:40, Jan Beulich wrote:
>>> --- a/xen/arch/x86/cpu/mtrr/main.c
>>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>>> printk(KERN_WARNING
>>> "mtrr: your processor doesn't support write-combining\n");
>>> - return -ENOSYS;
>>> + return -EOPNOTSUPP;
>> Will this break the classic-xen MTRR code? ISTR it was very picky, from
>> the cpuid work.
> There are no -ENOSYS checks in there afaics, and I also can't
> otherwise see any way for this change to break it.
>
>> Also, as some further cleanup, that printk should
>> become a print-once.
> Well, for a message that presumably would never actually get
> issued (as I'm unaware of 64-bit capable CPUs not supporting
> WC) I don't think this sort of cleanup has a really high priority.
> Certainly not in this patch.
Agreed. This was a TODO note, rather than a request for this patch. I
have noticed a few other printk()'s which should become print once.
Sorry for the confusion.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Ping: Re: [PATCH] replace bogus -ENOSYS uses
2016-08-12 11:02 ` Jan Beulich
@ 2016-08-17 13:40 ` Jan Beulich
0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2016-08-17 13:40 UTC (permalink / raw)
To: George Dunlap
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
>>> On 12.08.16 at 13:02, <JBeulich@suse.com> wrote:
Ping:
>>>> On 12.08.16 at 12:34, <george.dunlap@citrix.com> wrote:
>> On 11/08/16 19:10, Andrew Cooper wrote:
>>> On 09/08/16 11:40, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/cpu/mtrr/main.c
>>>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>>>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>>>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>>>> printk(KERN_WARNING
>>>> "mtrr: your processor doesn't support write-combining\n");
>>>> - return -ENOSYS;
>>>> + return -EOPNOTSUPP;
>>>
>>> Will this break the classic-xen MTRR code? ISTR it was very picky, from
>>> the cpuid work. Also, as some further cleanup, that printk should
>>> become a print-once.
>>>
>>> The others look ok.
>>
>> That does bring up a good general point though -- the return value is
>> part of the ABI. Are you reasonably confident that none of the callers
>> will be confused when this return value changes? If so, a note in the
>> commit message justifying this confidence would be helpful I think.
>
> I don't think specific return values can be considered part of the
> ABI, or else we couldn't e.g. change the order in which certain
> checks get performed.
>
> And then please also consider a hypothetical future hypervisor with
> the MTRR operations simply ripped out - that would return -ENOSYS
> or -EOPNOTSUPP then too, without a way for the caller to tell that
> more generic error condition from the more specific one here.
Does this address your concerns? I'm still hoping to get a formal
ack/nak on this one ...
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Ping: [PATCH] replace bogus -ENOSYS uses
2016-08-09 10:40 [PATCH] replace bogus -ENOSYS uses Jan Beulich
2016-08-11 18:10 ` Andrew Cooper
@ 2016-08-29 14:01 ` Jan Beulich
2016-09-05 13:20 ` George Dunlap
2 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2016-08-29 14:01 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu, George Dunlap, Ian Jackson,
Stefano Stabellini, Konrad Rzeszutek Wilk, Tim Deegan
Cc: xen-devel
>>> On 09.08.16 at 12:40, <JBeulich@suse.com> wrote:
> This doesn't cover all of them, just the ones that I think would most
> obviously better be -EINVAL or -EOPNOTSUPP.
Ping? There was a little bit of feedback, but non that really resulted in
a need to change something (so far at least).
Jan
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -440,7 +440,7 @@ int unmmap_broken_page(struct domain *d,
> return -EINVAL;
>
> if ( !has_hvm_container_domain(d) || !paging_mode_hap(d) )
> - return -ENOSYS;
> + return -EOPNOTSUPP;
>
> rc = -1;
> r_mfn = get_gfn_query(d, gfn, &pt);
> --- a/xen/arch/x86/cpu/mtrr/main.c
> +++ b/xen/arch/x86/cpu/mtrr/main.c
> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
> printk(KERN_WARNING
> "mtrr: your processor doesn't support write-combining\n");
> - return -ENOSYS;
> + return -EOPNOTSUPP;
> }
>
> if (!size) {
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5592,7 +5592,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> break;
>
> case HVMOP_flush_tlbs:
> - rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
> + rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -EINVAL;
> break;
>
> case HVMOP_track_dirty_vram:
> @@ -5832,7 +5832,7 @@ int hvm_debug_op(struct vcpu *v, int32_t
> {
> case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
> case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
> - rc = -ENOSYS;
> + rc = -EOPNOTSUPP;
> if ( !cpu_has_monitor_trap_flag )
> break;
> rc = 0;
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3565,7 +3565,7 @@ long do_mmuext_op(
> if ( !opt_allow_superpage )
> {
> MEM_LOG("Superpages disallowed");
> - rc = -ENOSYS;
> + rc = -EOPNOTSUPP;
> }
> else if ( unlikely(d != pg_owner) )
> rc = -EPERM;
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -353,7 +353,7 @@ int compat_memory_op(unsigned int cmd, X
> struct get_reserved_device_memory grdm;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&grdm.map, compat, 1) ||
> !compat_handle_okay(grdm.map.buffer, grdm.map.nr_entries)
> )
> --- a/xen/common/event_fifo.c
> +++ b/xen/common/event_fifo.c
> @@ -621,7 +621,7 @@ int evtchn_fifo_expand_array(const struc
> int rc;
>
> if ( !d->evtchn_fifo )
> - return -ENOSYS;
> + return -EOPNOTSUPP;
>
> spin_lock(&d->event_lock);
> rc = add_page_to_event_array(d, expand_array->array_gfn);
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3025,7 +3025,7 @@ do_grant_table_op(
> return -EINVAL;
>
> if ( (cmd &= GNTTABOP_CMD_MASK) != GNTTABOP_cache_flush && opaque_in )
> - return -ENOSYS;
> + return -EINVAL;
>
> rc = -EFAULT;
> switch ( cmd )
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -937,14 +937,14 @@ long do_memory_op(unsigned long cmd, XEN
>
> case XENMEM_exchange:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> rc = memory_exchange(guest_handle_cast(arg,
> xen_memory_exchange_t));
> break;
>
> case XENMEM_maximum_ram_page:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> rc = max_page;
> break;
> @@ -953,7 +953,7 @@ long do_memory_op(unsigned long cmd, XEN
> case XENMEM_maximum_reservation:
> case XENMEM_maximum_gpfn:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&domid, arg, 1) )
> return -EFAULT;
> @@ -1077,7 +1077,7 @@ long do_memory_op(unsigned long cmd, XEN
> struct page_info *page;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&xrfp, arg, 1) )
> return -EFAULT;
> @@ -1114,7 +1114,7 @@ long do_memory_op(unsigned long cmd, XEN
>
> case XENMEM_claim_pages:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&reservation, arg, 1) )
> return -EFAULT;
> @@ -1148,7 +1148,7 @@ long do_memory_op(unsigned long cmd, XEN
> struct vnuma_info tmp;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> /*
> * Guest passes nr_vnodes, number of regions and nr_vcpus thus
> @@ -1280,7 +1280,7 @@ long do_memory_op(unsigned long cmd, XEN
> struct get_reserved_device_memory grdm;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&grdm.map, arg, 1) ||
> !guest_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-09 10:40 [PATCH] replace bogus -ENOSYS uses Jan Beulich
2016-08-11 18:10 ` Andrew Cooper
2016-08-29 14:01 ` Ping: " Jan Beulich
@ 2016-09-05 13:20 ` George Dunlap
2 siblings, 0 replies; 11+ messages in thread
From: George Dunlap @ 2016-09-05 13:20 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Ian Jackson, Tim Deegan,
Andrew Cooper, xen-devel
On Tue, Aug 9, 2016 at 11:40 AM, Jan Beulich <JBeulich@suse.com> wrote:
> This doesn't cover all of them, just the ones that I think would most
> obviously better be -EINVAL or -EOPNOTSUPP.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
FWIW:
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
>
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -440,7 +440,7 @@ int unmmap_broken_page(struct domain *d,
> return -EINVAL;
>
> if ( !has_hvm_container_domain(d) || !paging_mode_hap(d) )
> - return -ENOSYS;
> + return -EOPNOTSUPP;
>
> rc = -1;
> r_mfn = get_gfn_query(d, gfn, &pt);
> --- a/xen/arch/x86/cpu/mtrr/main.c
> +++ b/xen/arch/x86/cpu/mtrr/main.c
> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
> printk(KERN_WARNING
> "mtrr: your processor doesn't support write-combining\n");
> - return -ENOSYS;
> + return -EOPNOTSUPP;
> }
>
> if (!size) {
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5592,7 +5592,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> break;
>
> case HVMOP_flush_tlbs:
> - rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
> + rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -EINVAL;
> break;
>
> case HVMOP_track_dirty_vram:
> @@ -5832,7 +5832,7 @@ int hvm_debug_op(struct vcpu *v, int32_t
> {
> case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
> case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
> - rc = -ENOSYS;
> + rc = -EOPNOTSUPP;
> if ( !cpu_has_monitor_trap_flag )
> break;
> rc = 0;
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3565,7 +3565,7 @@ long do_mmuext_op(
> if ( !opt_allow_superpage )
> {
> MEM_LOG("Superpages disallowed");
> - rc = -ENOSYS;
> + rc = -EOPNOTSUPP;
> }
> else if ( unlikely(d != pg_owner) )
> rc = -EPERM;
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -353,7 +353,7 @@ int compat_memory_op(unsigned int cmd, X
> struct get_reserved_device_memory grdm;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&grdm.map, compat, 1) ||
> !compat_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
> --- a/xen/common/event_fifo.c
> +++ b/xen/common/event_fifo.c
> @@ -621,7 +621,7 @@ int evtchn_fifo_expand_array(const struc
> int rc;
>
> if ( !d->evtchn_fifo )
> - return -ENOSYS;
> + return -EOPNOTSUPP;
>
> spin_lock(&d->event_lock);
> rc = add_page_to_event_array(d, expand_array->array_gfn);
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3025,7 +3025,7 @@ do_grant_table_op(
> return -EINVAL;
>
> if ( (cmd &= GNTTABOP_CMD_MASK) != GNTTABOP_cache_flush && opaque_in )
> - return -ENOSYS;
> + return -EINVAL;
>
> rc = -EFAULT;
> switch ( cmd )
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -937,14 +937,14 @@ long do_memory_op(unsigned long cmd, XEN
>
> case XENMEM_exchange:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> rc = memory_exchange(guest_handle_cast(arg, xen_memory_exchange_t));
> break;
>
> case XENMEM_maximum_ram_page:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> rc = max_page;
> break;
> @@ -953,7 +953,7 @@ long do_memory_op(unsigned long cmd, XEN
> case XENMEM_maximum_reservation:
> case XENMEM_maximum_gpfn:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&domid, arg, 1) )
> return -EFAULT;
> @@ -1077,7 +1077,7 @@ long do_memory_op(unsigned long cmd, XEN
> struct page_info *page;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&xrfp, arg, 1) )
> return -EFAULT;
> @@ -1114,7 +1114,7 @@ long do_memory_op(unsigned long cmd, XEN
>
> case XENMEM_claim_pages:
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&reservation, arg, 1) )
> return -EFAULT;
> @@ -1148,7 +1148,7 @@ long do_memory_op(unsigned long cmd, XEN
> struct vnuma_info tmp;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> /*
> * Guest passes nr_vnodes, number of regions and nr_vcpus thus
> @@ -1280,7 +1280,7 @@ long do_memory_op(unsigned long cmd, XEN
> struct get_reserved_device_memory grdm;
>
> if ( unlikely(start_extent) )
> - return -ENOSYS;
> + return -EINVAL;
>
> if ( copy_from_guest(&grdm.map, arg, 1) ||
> !guest_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-08-12 11:49 ` Andrew Cooper
@ 2016-09-06 8:03 ` Jan Beulich
2016-09-06 10:14 ` Andrew Cooper
0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2016-09-06 8:03 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, xen-devel
>>> On 12.08.16 at 13:49, <andrew.cooper3@citrix.com> wrote:
> On 12/08/16 11:58, Jan Beulich wrote:
>>>>> On 11.08.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
>>> On 09/08/16 11:40, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/cpu/mtrr/main.c
>>>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>>>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>>>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>>>> printk(KERN_WARNING
>>>> "mtrr: your processor doesn't support write-combining\n");
>>>> - return -ENOSYS;
>>>> + return -EOPNOTSUPP;
>>> Will this break the classic-xen MTRR code? ISTR it was very picky, from
>>> the cpuid work.
>> There are no -ENOSYS checks in there afaics, and I also can't
>> otherwise see any way for this change to break it.
>>
>>> Also, as some further cleanup, that printk should
>>> become a print-once.
>> Well, for a message that presumably would never actually get
>> issued (as I'm unaware of 64-bit capable CPUs not supporting
>> WC) I don't think this sort of cleanup has a really high priority.
>> Certainly not in this patch.
>
> Agreed. This was a TODO note, rather than a request for this patch. I
> have noticed a few other printk()'s which should become print once.
Btw., with the MTRR concern hopefully addressed, any chance of
getting an ack on the x86 pieces here?
Thanks, Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] replace bogus -ENOSYS uses
2016-09-06 8:03 ` Jan Beulich
@ 2016-09-06 10:14 ` Andrew Cooper
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Cooper @ 2016-09-06 10:14 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, xen-devel
On 06/09/16 09:03, Jan Beulich wrote:
>>>> On 12.08.16 at 13:49, <andrew.cooper3@citrix.com> wrote:
>> On 12/08/16 11:58, Jan Beulich wrote:
>>>>>> On 11.08.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
>>>> On 09/08/16 11:40, Jan Beulich wrote:
>>>>> --- a/xen/arch/x86/cpu/mtrr/main.c
>>>>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>>>>> @@ -332,7 +332,7 @@ int mtrr_add_page(unsigned long base, un
>>>>> if ((type == MTRR_TYPE_WRCOMB) && !have_wrcomb()) {
>>>>> printk(KERN_WARNING
>>>>> "mtrr: your processor doesn't support write-combining\n");
>>>>> - return -ENOSYS;
>>>>> + return -EOPNOTSUPP;
>>>> Will this break the classic-xen MTRR code? ISTR it was very picky, from
>>>> the cpuid work.
>>> There are no -ENOSYS checks in there afaics, and I also can't
>>> otherwise see any way for this change to break it.
>>>
>>>> Also, as some further cleanup, that printk should
>>>> become a print-once.
>>> Well, for a message that presumably would never actually get
>>> issued (as I'm unaware of 64-bit capable CPUs not supporting
>>> WC) I don't think this sort of cleanup has a really high priority.
>>> Certainly not in this patch.
>> Agreed. This was a TODO note, rather than a request for this patch. I
>> have noticed a few other printk()'s which should become print once.
> Btw., with the MTRR concern hopefully addressed, any chance of
> getting an ack on the x86 pieces here?
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-09-06 10:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 10:40 [PATCH] replace bogus -ENOSYS uses Jan Beulich
2016-08-11 18:10 ` Andrew Cooper
2016-08-12 10:34 ` George Dunlap
2016-08-12 11:02 ` Jan Beulich
2016-08-17 13:40 ` Ping: " Jan Beulich
2016-08-12 10:58 ` Jan Beulich
2016-08-12 11:49 ` Andrew Cooper
2016-09-06 8:03 ` Jan Beulich
2016-09-06 10:14 ` Andrew Cooper
2016-08-29 14:01 ` Ping: " Jan Beulich
2016-09-05 13:20 ` George Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.