| Message ID | 1741980536-3865-2-git-send-email-nunodasneves@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Introduce /dev/mshv root partition driver | expand |
From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, March 14, 2025 12:29 PM > > Introduce hv_status_printk() macros as a convenience to log hypercall > errors, formatting them with the status code (HV_STATUS_*) as a raw hex > value and also as a string, which saves some time while debugging. > > Create a table of HV_STATUS_ codes with strings and mapped errnos, and > use it for hv_result_to_string() and hv_result_to_errno(). > > Use the new hv_status_printk()s in hv_proc.c, hyperv-iommu.c, and > irqdomain.c hypercalls to aid debugging in the root partition. > > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com> > Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> > --- > arch/x86/hyperv/irqdomain.c | 6 +- > drivers/hv/hv_common.c | 129 ++++++++++++++++++++++++--------- > drivers/hv/hv_proc.c | 10 +-- > drivers/iommu/hyperv-iommu.c | 4 +- > include/asm-generic/mshyperv.h | 13 ++++ > 5 files changed, 118 insertions(+), 44 deletions(-) > [snip] > + > +struct hv_status_info { > + char *string; > + int errno; > + u16 code; > +}; > + > +/* > + * Note on the errno mappings: > + * A failed hypercall is usually only recoverable (or loggable) near > + * the call site where the HV_STATUS_* code is known. So the errno > + * it gets converted to is not too useful further up the stack. > + * Provide a few mappings that could be useful, and revert to -EIO > + * as a fallback. > + */ > +static const struct hv_status_info hv_status_infos[] = { > +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } > + _STATUS_INFO(HV_STATUS_SUCCESS, 0), > + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), > + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), > + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), > + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), > + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), > + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), > + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), > + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), > + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), > + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), > + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), > + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), > + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), > + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), > + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), > + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), > +#undef _STATUS_INFO > +}; > + > +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) > +{ > + int i; > + u16 code = hv_result(hv_status); > + > + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { > + const struct hv_status_info *info = &hv_status_infos[i]; > + > + if (info->code == code) > + return info; > + } > + > + return NULL; > +} > + > +/* Convert a hypercall result into a linux-friendly error code. */ > +int hv_result_to_errno(u64 status) > +{ > + const struct hv_status_info *info; > + > + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ > + if (unlikely(status == U64_MAX)) > + return -EOPNOTSUPP; > + > + info = find_hv_status_info(status); > + if (info) > + return info->errno; > + > + return -EIO; > +} > +EXPORT_SYMBOL_GPL(hv_result_to_errno); > + > +const char *hv_result_to_string(u64 status) > +{ > + const struct hv_status_info *info; > + > + if (unlikely(status == U64_MAX)) > + return "Hypercall page missing!"; > + > + info = find_hv_status_info(status); > + if (info) > + return info->string; > + > + return "Unknown"; > +} > +EXPORT_SYMBOL_GPL(hv_result_to_string); I think the table-driven approach worked out pretty well. But here's a version that is even more compact, and avoids the duplicate testing for U64_MAX and having to special case both U64_MAX and not finding a match: + +struct hv_status_info { + char *string; + int errno; + int code; +}; + +/* + * Note on the errno mappings: + * A failed hypercall is usually only recoverable (or loggable) near + * the call site where the HV_STATUS_* code is known. So the errno + * it gets converted to is not too useful further up the stack. + * Provide a few mappings that could be useful, and revert to -EIO + * as a fallback. + */ +static const struct hv_status_info hv_status_infos[] = { +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } + _STATUS_INFO(HV_STATUS_SUCCESS, 0), + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), + {"Hypercall page missing!", -EOPNOTSUPP, -1}, /* code -1 is "no hypercall page" */ + {"Unknown", -EIO, -2}, /* code -2 is "Not found" entry; must be last */ +#undef _STATUS_INFO +}; + +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) +{ + int i, code; + const struct hv_status_info *info; + + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ + if (unlikely(hv_status == U64_MAX)) + code = -1; + else + code = hv_result(hv_status); + + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { + info = &hv_status_infos[i]; + if (info->code == code || info->code == -2) + break; + } + + return info; +} + +/* Convert a hypercall result into a linux-friendly error code. */ +int hv_result_to_errno(u64 status) +{ + return find_hv_status_info(status)->errno; +} +EXPORT_SYMBOL_GPL(hv_result_to_errno); + +const char *hv_result_to_string(u64 status) +{ + return find_hv_status_info(status)->string; +} +EXPORT_SYMBOL_GPL(hv_result_to_string); It could be even more compact by exporting find_hv_status_info() and letting hv_result_to_errno() and hv_result_to_string() be #defines to find_hv_status_info()->errno and find_hv_status_info()->string, respectively. Note that in struct hv_status_info, the "code" field is defined as "int" instead of "u16" so that it can contain sentinel values -1 and -2 that won't overlap with HV_STATUS_* values. Anyway, just a suggestion. The current code works from what I can see. Michael
On 3/14/2025 12:28 PM, Nuno Das Neves wrote: > Introduce hv_status_printk() macros as a convenience to log hypercall > errors, formatting them with the status code (HV_STATUS_*) as a raw hex > value and also as a string, which saves some time while debugging. > > Create a table of HV_STATUS_ codes with strings and mapped errnos, and > use it for hv_result_to_string() and hv_result_to_errno(). > > Use the new hv_status_printk()s in hv_proc.c, hyperv-iommu.c, and > irqdomain.c hypercalls to aid debugging in the root partition. > > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com> > Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> > --- > arch/x86/hyperv/irqdomain.c | 6 +- > drivers/hv/hv_common.c | 129 ++++++++++++++++++++++++--------- > drivers/hv/hv_proc.c | 10 +-- > drivers/iommu/hyperv-iommu.c | 4 +- > include/asm-generic/mshyperv.h | 13 ++++ > 5 files changed, 118 insertions(+), 44 deletions(-) > Looks good to me. Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>
On 3/18/2025 11:01 AM, Michael Kelley wrote: > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, March 14, 2025 12:29 PM >> >> Introduce hv_status_printk() macros as a convenience to log hypercall >> errors, formatting them with the status code (HV_STATUS_*) as a raw hex >> value and also as a string, which saves some time while debugging. >> >> Create a table of HV_STATUS_ codes with strings and mapped errnos, and >> use it for hv_result_to_string() and hv_result_to_errno(). >> >> Use the new hv_status_printk()s in hv_proc.c, hyperv-iommu.c, and >> irqdomain.c hypercalls to aid debugging in the root partition. >> >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com> >> Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> >> --- >> arch/x86/hyperv/irqdomain.c | 6 +- >> drivers/hv/hv_common.c | 129 ++++++++++++++++++++++++--------- >> drivers/hv/hv_proc.c | 10 +-- >> drivers/iommu/hyperv-iommu.c | 4 +- >> include/asm-generic/mshyperv.h | 13 ++++ >> 5 files changed, 118 insertions(+), 44 deletions(-) >> > > [snip] > >> + >> +struct hv_status_info { >> + char *string; >> + int errno; >> + u16 code; >> +}; >> + >> +/* >> + * Note on the errno mappings: >> + * A failed hypercall is usually only recoverable (or loggable) near >> + * the call site where the HV_STATUS_* code is known. So the errno >> + * it gets converted to is not too useful further up the stack. >> + * Provide a few mappings that could be useful, and revert to -EIO >> + * as a fallback. >> + */ >> +static const struct hv_status_info hv_status_infos[] = { >> +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } >> + _STATUS_INFO(HV_STATUS_SUCCESS, 0), >> + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), >> + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), >> + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), >> + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), >> + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), >> + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), >> + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), >> + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), >> + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), >> + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), >> + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), >> + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), >> + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), >> + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), >> + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), >> + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), >> + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), >> + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), >> + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), >> + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), >> + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), >> + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), >> +#undef _STATUS_INFO >> +}; >> + >> +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) >> +{ >> + int i; >> + u16 code = hv_result(hv_status); >> + >> + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { >> + const struct hv_status_info *info = &hv_status_infos[i]; >> + >> + if (info->code == code) >> + return info; >> + } >> + >> + return NULL; >> +} >> + >> +/* Convert a hypercall result into a linux-friendly error code. */ >> +int hv_result_to_errno(u64 status) >> +{ >> + const struct hv_status_info *info; >> + >> + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ >> + if (unlikely(status == U64_MAX)) >> + return -EOPNOTSUPP; >> + >> + info = find_hv_status_info(status); >> + if (info) >> + return info->errno; >> + >> + return -EIO; >> +} >> +EXPORT_SYMBOL_GPL(hv_result_to_errno); >> + >> +const char *hv_result_to_string(u64 status) >> +{ >> + const struct hv_status_info *info; >> + >> + if (unlikely(status == U64_MAX)) >> + return "Hypercall page missing!"; >> + >> + info = find_hv_status_info(status); >> + if (info) >> + return info->string; >> + >> + return "Unknown"; >> +} >> +EXPORT_SYMBOL_GPL(hv_result_to_string); > > I think the table-driven approach worked out pretty well. But here's a version that > is even more compact, and avoids the duplicate testing for U64_MAX and having > to special case both U64_MAX and not finding a match: > > + > +struct hv_status_info { > + char *string; > + int errno; > + int code; > +}; > + > +/* > + * Note on the errno mappings: > + * A failed hypercall is usually only recoverable (or loggable) near > + * the call site where the HV_STATUS_* code is known. So the errno > + * it gets converted to is not too useful further up the stack. > + * Provide a few mappings that could be useful, and revert to -EIO > + * as a fallback. > + */ > +static const struct hv_status_info hv_status_infos[] = { > +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } > + _STATUS_INFO(HV_STATUS_SUCCESS, 0), > + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), > + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), > + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), > + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), > + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), > + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), > + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), > + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), > + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), > + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), > + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), > + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), > + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), > + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), > + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), > + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), > + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), > + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), > + {"Hypercall page missing!", -EOPNOTSUPP, -1}, /* code -1 is "no hypercall page" */ > + {"Unknown", -EIO, -2}, /* code -2 is "Not found" entry; must be last */ > +#undef _STATUS_INFO > +}; > + > +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) > +{ > + int i, code; > + const struct hv_status_info *info; > + > + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ > + if (unlikely(hv_status == U64_MAX)) > + code = -1; > + else > + code = hv_result(hv_status); > + > + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { > + info = &hv_status_infos[i]; > + if (info->code == code || info->code == -2) > + break; > + } > + > + return info; > +} > + > +/* Convert a hypercall result into a linux-friendly error code. */ > +int hv_result_to_errno(u64 status) > +{ > + return find_hv_status_info(status)->errno; > +} > +EXPORT_SYMBOL_GPL(hv_result_to_errno); > + > +const char *hv_result_to_string(u64 status) > +{ > + return find_hv_status_info(status)->string; > +} > +EXPORT_SYMBOL_GPL(hv_result_to_string); > > It could be even more compact by exporting find_hv_status_info() and > letting hv_result_to_errno() and hv_result_to_string() be #defines to > find_hv_status_info()->errno and find_hv_status_info()->string, > respectively. > > Note that in struct hv_status_info, the "code" field is defined as "int" > instead of "u16" so that it can contain sentinel values -1 and -2 that > won't overlap with HV_STATUS_* values. > Played around with this some more. I like your idea of making it more compact by dealing with U64_MAX and unknown in find_hv_status_info(), however I'm not as keen on putting these cases in the array and iterating over the whole array when they could just be static constants or inline struct initializers. See below. I also like the idea of making hv_result_to_*() functions into simple macros and exporting find_hv_status_info(). However, if it gets used elsewhere it makes more sense if the returned hv_status_info for the "Unknown" case contains the actual status code instead of replacing that information with -2, so then I'd want to return it by value instead of pointer: +static const struct hv_status_info hv_status_infos[] = { +#define _STATUS_INFO(status, errno) { #status, (status), (errno) } + _STATUS_INFO(HV_STATUS_SUCCESS, 0), <snip> + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), +#undef _STATUS_INFO +}; + +struct hv_status_info hv_get_status_info(u64 hv_status) +{ + int i; + const struct hv_status_info *info; + u16 code = hv_result(hv_status); + struct hv_status_info ret = {"Unknown", code, -EIO}; + + if (hv_status == U64_MAX) + ret = (struct hv_status_info){"Hypercall page missing!", -1, + -EOPNOTSUPP}; + else + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { + info = &hv_status_infos[i]; + if (info->code == code) { + ret = *info; + break; + } + } + + return ret; +} +EXPORT_SYMBOL_GPL(hv_get_status_info); and in mshyperv.h: +#define hv_result_to_string(hv_status) hv_get_status_info(hv_status).string +#define hv_result_to_errno(hv_status) hv_get_status_info(hv_status).errno + +struct hv_status_info { + char *string; + int code; + int errno; +}; + +struct hv_status_info hv_get_status_info(u64 hv_status); Note also I switched the order of code and errno in hv_status_info, mainly because I think the struct initializers for "Unknown" and "Hypercall page missing!" are more readable with that order: {string, code, errno} Do you see any problems with the above? > Anyway, just a suggestion. The current code works from what I can > see. Thanks, it's not a bad idea at all to make it as compact and readable as possible on the first try, but not a big loss either way. Thanks Nuno > > Michael
From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, March 21, 2025 12:13 PM > > On 3/18/2025 11:01 AM, Michael Kelley wrote: > > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, March 14, 2025 12:29 PM > >> > >> Introduce hv_status_printk() macros as a convenience to log hypercall > >> errors, formatting them with the status code (HV_STATUS_*) as a raw hex > >> value and also as a string, which saves some time while debugging. > >> > >> Create a table of HV_STATUS_ codes with strings and mapped errnos, and > >> use it for hv_result_to_string() and hv_result_to_errno(). > >> > >> Use the new hv_status_printk()s in hv_proc.c, hyperv-iommu.c, and > >> irqdomain.c hypercalls to aid debugging in the root partition. > >> > >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com> > >> Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> > >> --- > >> arch/x86/hyperv/irqdomain.c | 6 +- > >> drivers/hv/hv_common.c | 129 ++++++++++++++++++++++++--------- > >> drivers/hv/hv_proc.c | 10 +-- > >> drivers/iommu/hyperv-iommu.c | 4 +- > >> include/asm-generic/mshyperv.h | 13 ++++ > >> 5 files changed, 118 insertions(+), 44 deletions(-) > >> > > > > [snip] > > > >> + > >> +struct hv_status_info { > >> + char *string; > >> + int errno; > >> + u16 code; > >> +}; > >> + > >> +/* > >> + * Note on the errno mappings: > >> + * A failed hypercall is usually only recoverable (or loggable) near > >> + * the call site where the HV_STATUS_* code is known. So the errno > >> + * it gets converted to is not too useful further up the stack. > >> + * Provide a few mappings that could be useful, and revert to -EIO > >> + * as a fallback. > >> + */ > >> +static const struct hv_status_info hv_status_infos[] = { > >> +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } > >> + _STATUS_INFO(HV_STATUS_SUCCESS, 0), > >> + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), > >> + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), > >> + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), > >> + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), > >> + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), > >> + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), > >> + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), > >> + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), > >> + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), > >> + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), > >> + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), > >> + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), > >> + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), > >> + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), > >> + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), > >> + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), > >> + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), > >> + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), > >> + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), > >> + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), > >> +#undef _STATUS_INFO > >> +}; > >> + > >> +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) > >> +{ > >> + int i; > >> + u16 code = hv_result(hv_status); > >> + > >> + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { > >> + const struct hv_status_info *info = &hv_status_infos[i]; > >> + > >> + if (info->code == code) > >> + return info; > >> + } > >> + > >> + return NULL; > >> +} > >> + > >> +/* Convert a hypercall result into a linux-friendly error code. */ > >> +int hv_result_to_errno(u64 status) > >> +{ > >> + const struct hv_status_info *info; > >> + > >> + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ > >> + if (unlikely(status == U64_MAX)) > >> + return -EOPNOTSUPP; > >> + > >> + info = find_hv_status_info(status); > >> + if (info) > >> + return info->errno; > >> + > >> + return -EIO; > >> +} > >> +EXPORT_SYMBOL_GPL(hv_result_to_errno); > >> + > >> +const char *hv_result_to_string(u64 status) > >> +{ > >> + const struct hv_status_info *info; > >> + > >> + if (unlikely(status == U64_MAX)) > >> + return "Hypercall page missing!"; > >> + > >> + info = find_hv_status_info(status); > >> + if (info) > >> + return info->string; > >> + > >> + return "Unknown"; > >> +} > >> +EXPORT_SYMBOL_GPL(hv_result_to_string); > > > > I think the table-driven approach worked out pretty well. But here's a version that > > is even more compact, and avoids the duplicate testing for U64_MAX and having > > to special case both U64_MAX and not finding a match: > > > > + > > +struct hv_status_info { > > + char *string; > > + int errno; > > + int code; > > +}; > > + > > +/* > > + * Note on the errno mappings: > > + * A failed hypercall is usually only recoverable (or loggable) near > > + * the call site where the HV_STATUS_* code is known. So the errno > > + * it gets converted to is not too useful further up the stack. > > + * Provide a few mappings that could be useful, and revert to -EIO > > + * as a fallback. > > + */ > > +static const struct hv_status_info hv_status_infos[] = { > > +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } > > + _STATUS_INFO(HV_STATUS_SUCCESS, 0), > > + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), > > + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), > > + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), > > + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), > > + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), > > + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), > > + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), > > + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), > > + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), > > + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), > > + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), > > + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), > > + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), > > + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), > > + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), > > + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), > > + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), > > + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), > > + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), > > + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), > > + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), > > + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), > > + {"Hypercall page missing!", -EOPNOTSUPP, -1}, /* code -1 is "no hypercall page" */ > > + {"Unknown", -EIO, -2}, /* code -2 is "Not found" entry; must be last */ > > +#undef _STATUS_INFO > > +}; > > + > > +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) > > +{ > > + int i, code; > > + const struct hv_status_info *info; > > + > > + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ > > + if (unlikely(hv_status == U64_MAX)) > > + code = -1; > > + else > > + code = hv_result(hv_status); > > + > > + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { > > + info = &hv_status_infos[i]; > > + if (info->code == code || info->code == -2) > > + break; > > + } > > + > > + return info; > > +} > > + > > +/* Convert a hypercall result into a linux-friendly error code. */ > > +int hv_result_to_errno(u64 status) > > +{ > > + return find_hv_status_info(status)->errno; > > +} > > +EXPORT_SYMBOL_GPL(hv_result_to_errno); > > + > > +const char *hv_result_to_string(u64 status) > > +{ > > + return find_hv_status_info(status)->string; > > +} > > +EXPORT_SYMBOL_GPL(hv_result_to_string); > > > > It could be even more compact by exporting find_hv_status_info() and > > letting hv_result_to_errno() and hv_result_to_string() be #defines to > > find_hv_status_info()->errno and find_hv_status_info()->string, > > respectively. > > > > Note that in struct hv_status_info, the "code" field is defined as "int" > > instead of "u16" so that it can contain sentinel values -1 and -2 that > > won't overlap with HV_STATUS_* values. > > > Played around with this some more. > > I like your idea of making it more compact by dealing with U64_MAX and > unknown in find_hv_status_info(), however I'm not as keen on putting > these cases in the array and iterating over the whole array when they > could just be static constants or inline struct initializers. See below. > > I also like the idea of making hv_result_to_*() functions into simple > macros and exporting find_hv_status_info(). However, if it gets used > elsewhere it makes more sense if the returned hv_status_info for the > "Unknown" case contains the actual status code instead of replacing > that information with -2, so then I'd want to return it by value > instead of pointer: OK, yes I agree that having the -2 in the status code field could be a little weird. > > +static const struct hv_status_info hv_status_infos[] = { > +#define _STATUS_INFO(status, errno) { #status, (status), (errno) } > + _STATUS_INFO(HV_STATUS_SUCCESS, 0), > <snip> > + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), > +#undef _STATUS_INFO > +}; > + > +struct hv_status_info hv_get_status_info(u64 hv_status) > +{ > + int i; > + const struct hv_status_info *info; > + u16 code = hv_result(hv_status); > + struct hv_status_info ret = {"Unknown", code, -EIO}; > + > + if (hv_status == U64_MAX) > + ret = (struct hv_status_info){"Hypercall page missing!", -1, > + -EOPNOTSUPP}; > + else > + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { > + info = &hv_status_infos[i]; > + if (info->code == code) { > + ret = *info; > + break; > + } > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(hv_get_status_info); > > and in mshyperv.h: > > +#define hv_result_to_string(hv_status) hv_get_status_info(hv_status).string > +#define hv_result_to_errno(hv_status) hv_get_status_info(hv_status).errno > + > +struct hv_status_info { > + char *string; > + int code; > + int errno; > +}; > + > +struct hv_status_info hv_get_status_info(u64 hv_status); > > Note also I switched the order of code and errno in hv_status_info, > mainly because I think the struct initializers for "Unknown" and > "Hypercall page missing!" are more readable with that order: > {string, code, errno} > > Do you see any problems with the above? I don't see any problems. Having struct hv_status_info assigned and returned by value is a little out-of-the-ordinary since it is bigger than 64 bits. But it is only 128 bits so should be OK. If you want to adopt your new version as a replacement for the original version, I'm good with that. Michael
diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c index 64b921360b0f..31f0d29cbc5e 100644 --- a/arch/x86/hyperv/irqdomain.c +++ b/arch/x86/hyperv/irqdomain.c @@ -64,7 +64,7 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level, local_irq_restore(flags); if (!hv_result_success(status)) - pr_err("%s: hypercall failed, status %lld\n", __func__, status); + hv_status_err(status, "\n"); return hv_result(status); } @@ -224,7 +224,7 @@ static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) kfree(stored_entry); if (status != HV_STATUS_SUCCESS) { - pr_debug("%s: failed to unmap, status %lld", __func__, status); + hv_status_debug(status, "failed to unmap\n"); return; } } @@ -273,7 +273,7 @@ static void hv_teardown_msi_irq(struct pci_dev *dev, struct irq_data *irqd) status = hv_unmap_msi_interrupt(dev, &old_entry); if (status != HV_STATUS_SUCCESS) - pr_err("%s: hypercall failed, status %lld\n", __func__, status); + hv_status_err(status, "\n"); } static void hv_msi_free_irq(struct irq_domain *domain, diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index 9804adb4cc56..885bbc3d86d8 100644 --- a/drivers/hv/hv_common.c +++ b/drivers/hv/hv_common.c @@ -684,40 +684,6 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2) } EXPORT_SYMBOL_GPL(hv_tdx_hypercall); -/* Convert a hypercall result into a linux-friendly error code. */ -int hv_result_to_errno(u64 status) -{ - /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ - if (unlikely(status == U64_MAX)) - return -EOPNOTSUPP; - /* - * A failed hypercall is usually only recoverable (or loggable) near - * the call site where the HV_STATUS_* code is known. So the errno - * it gets converted to is not too useful further up the stack. - * Provide a few mappings that could be useful, and revert to -EIO - * as a fallback. - */ - switch (hv_result(status)) { - case HV_STATUS_SUCCESS: - return 0; - case HV_STATUS_INVALID_HYPERCALL_CODE: - case HV_STATUS_INVALID_HYPERCALL_INPUT: - case HV_STATUS_INVALID_PARAMETER: - case HV_STATUS_INVALID_PARTITION_ID: - case HV_STATUS_INVALID_VP_INDEX: - case HV_STATUS_INVALID_PORT_ID: - case HV_STATUS_INVALID_CONNECTION_ID: - case HV_STATUS_INVALID_LP_INDEX: - case HV_STATUS_INVALID_REGISTER_VALUE: - return -EINVAL; - case HV_STATUS_INSUFFICIENT_MEMORY: - return -ENOMEM; - default: - break; - } - return -EIO; -} - void hv_identify_partition_type(void) { /* Assume guest role */ @@ -740,3 +706,98 @@ void hv_identify_partition_type(void) pr_crit("Hyper-V: CONFIG_MSHV_ROOT not enabled!\n"); } } + +struct hv_status_info { + char *string; + int errno; + u16 code; +}; + +/* + * Note on the errno mappings: + * A failed hypercall is usually only recoverable (or loggable) near + * the call site where the HV_STATUS_* code is known. So the errno + * it gets converted to is not too useful further up the stack. + * Provide a few mappings that could be useful, and revert to -EIO + * as a fallback. + */ +static const struct hv_status_info hv_status_infos[] = { +#define _STATUS_INFO(status, errno) { #status, (errno), (status) } + _STATUS_INFO(HV_STATUS_SUCCESS, 0), + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL), + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO), + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO), + _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO), + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO), + _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM), + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL), + _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL), + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO), + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO), + _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO), + _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL), + _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO), + _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO), + _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO), + _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO), + _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO), + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO), +#undef _STATUS_INFO +}; + +static inline const struct hv_status_info *find_hv_status_info(u64 hv_status) +{ + int i; + u16 code = hv_result(hv_status); + + for (i = 0; i < ARRAY_SIZE(hv_status_infos); ++i) { + const struct hv_status_info *info = &hv_status_infos[i]; + + if (info->code == code) + return info; + } + + return NULL; +} + +/* Convert a hypercall result into a linux-friendly error code. */ +int hv_result_to_errno(u64 status) +{ + const struct hv_status_info *info; + + /* hv_do_hypercall() may return U64_MAX, hypercalls aren't possible */ + if (unlikely(status == U64_MAX)) + return -EOPNOTSUPP; + + info = find_hv_status_info(status); + if (info) + return info->errno; + + return -EIO; +} +EXPORT_SYMBOL_GPL(hv_result_to_errno); + +const char *hv_result_to_string(u64 status) +{ + const struct hv_status_info *info; + + if (unlikely(status == U64_MAX)) + return "Hypercall page missing!"; + + info = find_hv_status_info(status); + if (info) + return info->string; + + return "Unknown"; +} +EXPORT_SYMBOL_GPL(hv_result_to_string); diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c index 2fae18e4f7d2..605999f10e17 100644 --- a/drivers/hv/hv_proc.c +++ b/drivers/hv/hv_proc.c @@ -87,7 +87,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages) page_count, 0, input_page, NULL); local_irq_restore(flags); if (!hv_result_success(status)) { - pr_err("Failed to deposit pages: %lld\n", status); + hv_status_err(status, "\n"); ret = hv_result_to_errno(status); goto err_free_allocations; } @@ -137,8 +137,8 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id) if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) { if (!hv_result_success(status)) { - pr_err("%s: cpu %u apic ID %u, %lld\n", __func__, - lp_index, apic_id, status); + hv_status_err(status, "cpu %u apic ID: %u\n", + lp_index, apic_id); ret = hv_result_to_errno(status); } break; @@ -179,8 +179,8 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags) if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) { if (!hv_result_success(status)) { - pr_err("%s: vcpu %u, lp %u, %lld\n", __func__, - vp_index, flags, status); + hv_status_err(status, "vcpu: %u, lp: %u\n", + vp_index, flags); ret = hv_result_to_errno(status); } break; diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c index 53e4b37716af..761ab647f372 100644 --- a/drivers/iommu/hyperv-iommu.c +++ b/drivers/iommu/hyperv-iommu.c @@ -217,7 +217,7 @@ hyperv_root_ir_compose_msi_msg(struct irq_data *irq_data, struct msi_msg *msg) status = hv_unmap_ioapic_interrupt(ioapic_id, &entry); if (status != HV_STATUS_SUCCESS) - pr_debug("%s: unexpected unmap status %lld\n", __func__, status); + hv_status_debug(status, "failed to unmap\n"); data->entry.ioapic_rte.as_uint64 = 0; data->entry.source = 0; /* Invalid source */ @@ -228,7 +228,7 @@ hyperv_root_ir_compose_msi_msg(struct irq_data *irq_data, struct msi_msg *msg) vector, &entry); if (status != HV_STATUS_SUCCESS) { - pr_err("%s: map hypercall failed, status %lld\n", __func__, status); + hv_status_err(status, "map failed\n"); return; } diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h index b13b0cda4ac8..250c65236919 100644 --- a/include/asm-generic/mshyperv.h +++ b/include/asm-generic/mshyperv.h @@ -298,6 +298,19 @@ static inline int cpumask_to_vpset_skip(struct hv_vpset *vpset, return __cpumask_to_vpset(vpset, cpus, func); } +#define _hv_status_fmt(fmt) "%s: Hyper-V status: %#x = %s: " fmt +#define hv_status_printk(level, status, fmt, ...) \ +do { \ + u64 __status = (status); \ + pr_##level(_hv_status_fmt(fmt), __func__, hv_result(__status), \ + hv_result_to_string(__status), ##__VA_ARGS__); \ +} while (0) +#define hv_status_err(status, fmt, ...) \ + hv_status_printk(err, status, fmt, ##__VA_ARGS__) +#define hv_status_debug(status, fmt, ...) \ + hv_status_printk(debug, status, fmt, ##__VA_ARGS__) + +const char *hv_result_to_string(u64 hv_status); int hv_result_to_errno(u64 status); void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die); bool hv_is_hyperv_initialized(void);