* [PATCH V5 1/6] vfio: platform: rename reset function
2016-05-16 2:13 [PATCH V5 0/6] vfio, platform: add ACPI support Sinan Kaya
@ 2016-05-16 2:13 ` Sinan Kaya
2016-05-23 12:52 ` Eric Auger
2016-05-16 2:13 ` [PATCH V5 2/6] vfio: platform: move reset call to a common function Sinan Kaya
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-05-16 2:13 UTC (permalink / raw)
To: linux-arm-kernel
Renaming the reset function to of_reset as it is only used
by the device tree based platforms.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 30 +++++++++++++--------------
drivers/vfio/platform/vfio_platform_private.h | 6 +++---
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index e65b142..08fd7c2 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -41,7 +41,7 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
if (!strcmp(iter->compat, compat) &&
try_module_get(iter->owner)) {
*module = iter->owner;
- reset_fn = iter->reset;
+ reset_fn = iter->of_reset;
break;
}
}
@@ -51,18 +51,18 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
{
- vdev->reset = vfio_platform_lookup_reset(vdev->compat,
- &vdev->reset_module);
- if (!vdev->reset) {
+ vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
+ &vdev->reset_module);
+ if (!vdev->of_reset) {
request_module("vfio-reset:%s", vdev->compat);
- vdev->reset = vfio_platform_lookup_reset(vdev->compat,
- &vdev->reset_module);
+ vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
+ &vdev->reset_module);
}
}
static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
{
- if (vdev->reset)
+ if (vdev->of_reset)
module_put(vdev->reset_module);
}
@@ -141,9 +141,9 @@ static void vfio_platform_release(void *device_data)
mutex_lock(&driver_lock);
if (!(--vdev->refcnt)) {
- if (vdev->reset) {
+ if (vdev->of_reset) {
dev_info(vdev->device, "reset\n");
- vdev->reset(vdev);
+ vdev->of_reset(vdev);
} else {
dev_warn(vdev->device, "no reset function found!\n");
}
@@ -175,9 +175,9 @@ static int vfio_platform_open(void *device_data)
if (ret)
goto err_irq;
- if (vdev->reset) {
+ if (vdev->of_reset) {
dev_info(vdev->device, "reset\n");
- vdev->reset(vdev);
+ vdev->of_reset(vdev);
} else {
dev_warn(vdev->device, "no reset function found!\n");
}
@@ -213,7 +213,7 @@ static long vfio_platform_ioctl(void *device_data,
if (info.argsz < minsz)
return -EINVAL;
- if (vdev->reset)
+ if (vdev->of_reset)
vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
info.flags = vdev->flags;
info.num_regions = vdev->num_regions;
@@ -312,8 +312,8 @@ static long vfio_platform_ioctl(void *device_data,
return ret;
} else if (cmd == VFIO_DEVICE_RESET) {
- if (vdev->reset)
- return vdev->reset(vdev);
+ if (vdev->of_reset)
+ return vdev->of_reset(vdev);
else
return -EINVAL;
}
@@ -611,7 +611,7 @@ void vfio_platform_unregister_reset(const char *compat,
mutex_lock(&driver_lock);
list_for_each_entry_safe(iter, temp, &reset_list, link) {
- if (!strcmp(iter->compat, compat) && (iter->reset == fn)) {
+ if (!strcmp(iter->compat, compat) && (iter->of_reset == fn)) {
list_del(&iter->link);
break;
}
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 42816dd..71ed7d1 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -71,7 +71,7 @@ struct vfio_platform_device {
struct resource*
(*get_resource)(struct vfio_platform_device *vdev, int i);
int (*get_irq)(struct vfio_platform_device *vdev, int i);
- int (*reset)(struct vfio_platform_device *vdev);
+ int (*of_reset)(struct vfio_platform_device *vdev);
};
typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
@@ -80,7 +80,7 @@ struct vfio_platform_reset_node {
struct list_head link;
char *compat;
struct module *owner;
- vfio_platform_reset_fn_t reset;
+ vfio_platform_reset_fn_t of_reset;
};
extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
@@ -103,7 +103,7 @@ extern void vfio_platform_unregister_reset(const char *compat,
static struct vfio_platform_reset_node __reset ## _node = { \
.owner = THIS_MODULE, \
.compat = __compat, \
- .reset = __reset, \
+ .of_reset = __reset, \
}; \
__vfio_platform_register_reset(&__reset ## _node)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V5 1/6] vfio: platform: rename reset function
2016-05-16 2:13 ` [PATCH V5 1/6] vfio: platform: rename reset function Sinan Kaya
@ 2016-05-23 12:52 ` Eric Auger
0 siblings, 0 replies; 19+ messages in thread
From: Eric Auger @ 2016-05-23 12:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sinan
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> Renaming the reset function to of_reset as it is only used
> by the device tree based platforms.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 30 +++++++++++++--------------
> drivers/vfio/platform/vfio_platform_private.h | 6 +++---
> 2 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index e65b142..08fd7c2 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -41,7 +41,7 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
> if (!strcmp(iter->compat, compat) &&
> try_module_get(iter->owner)) {
> *module = iter->owner;
> - reset_fn = iter->reset;
> + reset_fn = iter->of_reset;
> break;
> }
> }
> @@ -51,18 +51,18 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
>
> static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> {
> - vdev->reset = vfio_platform_lookup_reset(vdev->compat,
> - &vdev->reset_module);
> - if (!vdev->reset) {
> + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> + &vdev->reset_module);
> + if (!vdev->of_reset) {
> request_module("vfio-reset:%s", vdev->compat);
> - vdev->reset = vfio_platform_lookup_reset(vdev->compat,
> - &vdev->reset_module);
> + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> + &vdev->reset_module);
> }
> }
>
> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> {
> - if (vdev->reset)
> + if (vdev->of_reset)
> module_put(vdev->reset_module);
> }
>
> @@ -141,9 +141,9 @@ static void vfio_platform_release(void *device_data)
> mutex_lock(&driver_lock);
>
> if (!(--vdev->refcnt)) {
> - if (vdev->reset) {
> + if (vdev->of_reset) {
> dev_info(vdev->device, "reset\n");
> - vdev->reset(vdev);
> + vdev->of_reset(vdev);
> } else {
> dev_warn(vdev->device, "no reset function found!\n");
> }
> @@ -175,9 +175,9 @@ static int vfio_platform_open(void *device_data)
> if (ret)
> goto err_irq;
>
> - if (vdev->reset) {
> + if (vdev->of_reset) {
> dev_info(vdev->device, "reset\n");
> - vdev->reset(vdev);
> + vdev->of_reset(vdev);
> } else {
> dev_warn(vdev->device, "no reset function found!\n");
> }
> @@ -213,7 +213,7 @@ static long vfio_platform_ioctl(void *device_data,
> if (info.argsz < minsz)
> return -EINVAL;
>
> - if (vdev->reset)
> + if (vdev->of_reset)
> vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
> info.flags = vdev->flags;
> info.num_regions = vdev->num_regions;
> @@ -312,8 +312,8 @@ static long vfio_platform_ioctl(void *device_data,
> return ret;
>
> } else if (cmd == VFIO_DEVICE_RESET) {
> - if (vdev->reset)
> - return vdev->reset(vdev);
> + if (vdev->of_reset)
> + return vdev->of_reset(vdev);
> else
> return -EINVAL;
> }
> @@ -611,7 +611,7 @@ void vfio_platform_unregister_reset(const char *compat,
>
> mutex_lock(&driver_lock);
> list_for_each_entry_safe(iter, temp, &reset_list, link) {
> - if (!strcmp(iter->compat, compat) && (iter->reset == fn)) {
> + if (!strcmp(iter->compat, compat) && (iter->of_reset == fn)) {
> list_del(&iter->link);
> break;
> }
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 42816dd..71ed7d1 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -71,7 +71,7 @@ struct vfio_platform_device {
> struct resource*
> (*get_resource)(struct vfio_platform_device *vdev, int i);
> int (*get_irq)(struct vfio_platform_device *vdev, int i);
> - int (*reset)(struct vfio_platform_device *vdev);
> + int (*of_reset)(struct vfio_platform_device *vdev);
> };
>
> typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
> @@ -80,7 +80,7 @@ struct vfio_platform_reset_node {
> struct list_head link;
> char *compat;
> struct module *owner;
> - vfio_platform_reset_fn_t reset;
> + vfio_platform_reset_fn_t of_reset;
> };
>
> extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> @@ -103,7 +103,7 @@ extern void vfio_platform_unregister_reset(const char *compat,
> static struct vfio_platform_reset_node __reset ## _node = { \
> .owner = THIS_MODULE, \
> .compat = __compat, \
> - .reset = __reset, \
> + .of_reset = __reset, \
> }; \
> __vfio_platform_register_reset(&__reset ## _node)
>
>
Reviewed-by: Eric Auger <eric.auger@linaro.org>
Best Regards
Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V5 2/6] vfio: platform: move reset call to a common function
2016-05-16 2:13 [PATCH V5 0/6] vfio, platform: add ACPI support Sinan Kaya
2016-05-16 2:13 ` [PATCH V5 1/6] vfio: platform: rename reset function Sinan Kaya
@ 2016-05-16 2:13 ` Sinan Kaya
2016-05-23 13:02 ` Eric Auger
2016-05-16 2:13 ` [PATCH V5 3/6] vfio: platform: determine reset capability Sinan Kaya
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-05-16 2:13 UTC (permalink / raw)
To: linux-arm-kernel
The reset call sequence seems to replicate itself multiple times
across the file. Grouping them together for maintenance reasons.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 31 ++++++++++++++--------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 08fd7c2..cb91dd3 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -134,6 +134,18 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
kfree(vdev->regions);
}
+static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
+{
+ if (vdev->of_reset) {
+ dev_info(vdev->device, "reset\n");
+ vdev->of_reset(vdev);
+ return 0;
+ }
+
+ dev_warn(vdev->device, "no reset function found!\n");
+ return -EINVAL;
+}
+
static void vfio_platform_release(void *device_data)
{
struct vfio_platform_device *vdev = device_data;
@@ -141,12 +153,7 @@ static void vfio_platform_release(void *device_data)
mutex_lock(&driver_lock);
if (!(--vdev->refcnt)) {
- if (vdev->of_reset) {
- dev_info(vdev->device, "reset\n");
- vdev->of_reset(vdev);
- } else {
- dev_warn(vdev->device, "no reset function found!\n");
- }
+ vfio_platform_call_reset(vdev);
vfio_platform_regions_cleanup(vdev);
vfio_platform_irq_cleanup(vdev);
}
@@ -175,12 +182,7 @@ static int vfio_platform_open(void *device_data)
if (ret)
goto err_irq;
- if (vdev->of_reset) {
- dev_info(vdev->device, "reset\n");
- vdev->of_reset(vdev);
- } else {
- dev_warn(vdev->device, "no reset function found!\n");
- }
+ vfio_platform_call_reset(vdev);
}
vdev->refcnt++;
@@ -312,10 +314,7 @@ static long vfio_platform_ioctl(void *device_data,
return ret;
} else if (cmd == VFIO_DEVICE_RESET) {
- if (vdev->of_reset)
- return vdev->of_reset(vdev);
- else
- return -EINVAL;
+ return vfio_platform_call_reset(vdev);
}
return -ENOTTY;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V5 2/6] vfio: platform: move reset call to a common function
2016-05-16 2:13 ` [PATCH V5 2/6] vfio: platform: move reset call to a common function Sinan Kaya
@ 2016-05-23 13:02 ` Eric Auger
2016-05-24 2:14 ` Sinan Kaya
0 siblings, 1 reply; 19+ messages in thread
From: Eric Auger @ 2016-05-23 13:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sinan,
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> The reset call sequence seems to replicate itself multiple times
> across the file. Grouping them together for maintenance reasons.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 31 ++++++++++++++--------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index 08fd7c2..cb91dd3 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -134,6 +134,18 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
> kfree(vdev->regions);
> }
>
> +static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
> +{
> + if (vdev->of_reset) {
> + dev_info(vdev->device, "reset\n");
> + vdev->of_reset(vdev);
> + return 0;
you should return vdev->of_reset(vdev) to keep the existing
VFIO_DEVICE_RESET ioctl behavior
Once fixed, Reviewed-by: Eric Auger <eric.auger@linaro.org>
Besides, but this goes beyond the scope of your series, maybe we should
reconsider in the future what happens in case the reset fails on
open/release.
Best Regards
Eric
> + }
> +
> + dev_warn(vdev->device, "no reset function found!\n");
> + return -EINVAL;
> +}
> +
> static void vfio_platform_release(void *device_data)
> {
> struct vfio_platform_device *vdev = device_data;
> @@ -141,12 +153,7 @@ static void vfio_platform_release(void *device_data)
> mutex_lock(&driver_lock);
>
> if (!(--vdev->refcnt)) {
> - if (vdev->of_reset) {
> - dev_info(vdev->device, "reset\n");
> - vdev->of_reset(vdev);
> - } else {
> - dev_warn(vdev->device, "no reset function found!\n");
> - }
> + vfio_platform_call_reset(vdev);
> vfio_platform_regions_cleanup(vdev);
> vfio_platform_irq_cleanup(vdev);
> }
> @@ -175,12 +182,7 @@ static int vfio_platform_open(void *device_data)
> if (ret)
> goto err_irq;
>
> - if (vdev->of_reset) {
> - dev_info(vdev->device, "reset\n");
> - vdev->of_reset(vdev);
> - } else {
> - dev_warn(vdev->device, "no reset function found!\n");
> - }
> + vfio_platform_call_reset(vdev);
> }
>
> vdev->refcnt++;
> @@ -312,10 +314,7 @@ static long vfio_platform_ioctl(void *device_data,
> return ret;
>
> } else if (cmd == VFIO_DEVICE_RESET) {
> - if (vdev->of_reset)
> - return vdev->of_reset(vdev);
> - else
> - return -EINVAL;
> + return vfio_platform_call_reset(vdev);
> }
>
> return -ENOTTY;
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH V5 2/6] vfio: platform: move reset call to a common function
2016-05-23 13:02 ` Eric Auger
@ 2016-05-24 2:14 ` Sinan Kaya
0 siblings, 0 replies; 19+ messages in thread
From: Sinan Kaya @ 2016-05-24 2:14 UTC (permalink / raw)
To: linux-arm-kernel
On 5/23/2016 9:02 AM, Eric Auger wrote:
> Hi Sinan,
> On 05/16/2016 04:13 AM, Sinan Kaya wrote:
>> The reset call sequence seems to replicate itself multiple times
>> across the file. Grouping them together for maintenance reasons.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 31 ++++++++++++++--------------
>> 1 file changed, 15 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index 08fd7c2..cb91dd3 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -134,6 +134,18 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev)
>> kfree(vdev->regions);
>> }
>>
>> +static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
>> +{
>> + if (vdev->of_reset) {
>> + dev_info(vdev->device, "reset\n");
>> + vdev->of_reset(vdev);
>> + return 0;
> you should return vdev->of_reset(vdev) to keep the existing
> VFIO_DEVICE_RESET ioctl behavior
will do.
>
> Once fixed, Reviewed-by: Eric Auger <eric.auger@linaro.org>
>
thanks.
> Besides, but this goes beyond the scope of your series, maybe we should
> reconsider in the future what happens in case the reset fails on
> open/release.
>
I like giving an error immediately to be honest on open.
> Best Regards
>
> Eric
>> + }
>> +
>> + dev_warn(vdev->device, "no reset function found!\n");
>> + return -EINVAL;
>> +}
>> +
>> static void vfio_platform_release(void *device_data)
>> {
>> struct vfio_platform_device *vdev = device_data;
>> @@ -141,12 +153,7 @@ static void vfio_platform_release(void *device_data)
>> mutex_lock(&driver_lock);
>>
>> if (!(--vdev->refcnt)) {
>> - if (vdev->of_reset) {
>> - dev_info(vdev->device, "reset\n");
>> - vdev->of_reset(vdev);
>> - } else {
>> - dev_warn(vdev->device, "no reset function found!\n");
>> - }
>> + vfio_platform_call_reset(vdev);
>> vfio_platform_regions_cleanup(vdev);
>> vfio_platform_irq_cleanup(vdev);
>> }
>> @@ -175,12 +182,7 @@ static int vfio_platform_open(void *device_data)
>> if (ret)
>> goto err_irq;
>>
>> - if (vdev->of_reset) {
>> - dev_info(vdev->device, "reset\n");
>> - vdev->of_reset(vdev);
>> - } else {
>> - dev_warn(vdev->device, "no reset function found!\n");
>> - }
>> + vfio_platform_call_reset(vdev);
>> }
>>
>> vdev->refcnt++;
>> @@ -312,10 +314,7 @@ static long vfio_platform_ioctl(void *device_data,
>> return ret;
>>
>> } else if (cmd == VFIO_DEVICE_RESET) {
>> - if (vdev->of_reset)
>> - return vdev->of_reset(vdev);
>> - else
>> - return -EINVAL;
>> + return vfio_platform_call_reset(vdev);
>> }
>>
>> return -ENOTTY;
>>
>
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V5 3/6] vfio: platform: determine reset capability
2016-05-16 2:13 [PATCH V5 0/6] vfio, platform: add ACPI support Sinan Kaya
2016-05-16 2:13 ` [PATCH V5 1/6] vfio: platform: rename reset function Sinan Kaya
2016-05-16 2:13 ` [PATCH V5 2/6] vfio: platform: move reset call to a common function Sinan Kaya
@ 2016-05-16 2:13 ` Sinan Kaya
2016-05-23 13:16 ` Eric Auger
2016-05-16 2:13 ` [PATCH V5 4/6] vfio: platform: add support for ACPI probe Sinan Kaya
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-05-16 2:13 UTC (permalink / raw)
To: linux-arm-kernel
Creating a new function to determine if this driver supports reset
function or not. This is an attempt to abstract device tree calls
from the rest of the code.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index cb91dd3..25378bd 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -49,6 +49,11 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
return reset_fn;
}
+static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
+{
+ return vdev->of_reset ? true : false;
+}
+
static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
{
vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
@@ -215,7 +220,7 @@ static long vfio_platform_ioctl(void *device_data,
if (info.argsz < minsz)
return -EINVAL;
- if (vdev->of_reset)
+ if (vfio_platform_has_reset(vdev))
vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
info.flags = vdev->flags;
info.num_regions = vdev->num_regions;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V5 3/6] vfio: platform: determine reset capability
2016-05-16 2:13 ` [PATCH V5 3/6] vfio: platform: determine reset capability Sinan Kaya
@ 2016-05-23 13:16 ` Eric Auger
0 siblings, 0 replies; 19+ messages in thread
From: Eric Auger @ 2016-05-23 13:16 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> Creating a new function to determine if this driver supports reset
> function or not. This is an attempt to abstract device tree calls
> from the rest of the code.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index cb91dd3..25378bd 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -49,6 +49,11 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
> return reset_fn;
> }
>
> +static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
> +{
> + return vdev->of_reset ? true : false;
> +}
> +
> static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> {
> vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> @@ -215,7 +220,7 @@ static long vfio_platform_ioctl(void *device_data,
> if (info.argsz < minsz)
> return -EINVAL;
>
> - if (vdev->of_reset)
> + if (vfio_platform_has_reset(vdev))
> vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
> info.flags = vdev->flags;
> info.num_regions = vdev->num_regions;
>
Reviewed-by: Eric Auger <eric.auger@linaro.org>
Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V5 4/6] vfio: platform: add support for ACPI probe
2016-05-16 2:13 [PATCH V5 0/6] vfio, platform: add ACPI support Sinan Kaya
` (2 preceding siblings ...)
2016-05-16 2:13 ` [PATCH V5 3/6] vfio: platform: determine reset capability Sinan Kaya
@ 2016-05-16 2:13 ` Sinan Kaya
2016-05-23 13:18 ` Eric Auger
2016-05-16 2:13 ` [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI Sinan Kaya
2016-05-16 2:13 ` [PATCH V5 6/6] vfio, platform: make reset driver a requirement by default Sinan Kaya
5 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-05-16 2:13 UTC (permalink / raw)
To: linux-arm-kernel
The code is using the compatible DT string to associate a reset driver
with the actual device itself. The compatible string does not exist on
ACPI based systems. HID is the unique identifier for a device driver
instead.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 58 ++++++++++++++++++++++++---
drivers/vfio/platform/vfio_platform_private.h | 1 +
2 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 25378bd..d859d3b 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -13,6 +13,7 @@
*/
#include <linux/device.h>
+#include <linux/acpi.h>
#include <linux/iommu.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -49,6 +50,37 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
return reset_fn;
}
+#ifdef CONFIG_ACPI
+int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (acpi_disabled)
+ return -ENODEV;
+
+ if (!adev) {
+ pr_err("VFIO: ACPI companion device not found for %s\n",
+ vdev->name);
+ return -ENODEV;
+ }
+
+ vdev->acpihid = acpi_device_hid(adev);
+ if (!vdev->acpihid) {
+ pr_err("VFIO: cannot find ACPI HID for %s\n",
+ vdev->name);
+ return -ENODEV;
+ }
+ return 0;
+}
+#else
+int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ return -EINVAL;
+}
+#endif
+
static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
{
return vdev->of_reset ? true : false;
@@ -548,6 +580,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
.mmap = vfio_platform_mmap,
};
+int vfio_platform_of_probe(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ int ret;
+
+ ret = device_property_read_string(dev, "compatible",
+ &vdev->compat);
+ if (ret) {
+ pr_err("VFIO: cannot retrieve compat for %s\n",
+ vdev->name);
+ return ret;
+ }
+ return 0;
+}
+
int vfio_platform_probe_common(struct vfio_platform_device *vdev,
struct device *dev)
{
@@ -557,11 +604,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
if (!vdev)
return -EINVAL;
- ret = device_property_read_string(dev, "compatible", &vdev->compat);
- if (ret) {
- pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
- return -EINVAL;
- }
+ ret = vfio_platform_acpi_probe(vdev, dev);
+ if (ret)
+ ret = vfio_platform_of_probe(vdev, dev);
+
+ if (ret)
+ return ret;
vdev->device = dev;
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 71ed7d1..ba9e4f8 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -58,6 +58,7 @@ struct vfio_platform_device {
struct mutex igate;
struct module *parent_module;
const char *compat;
+ const char *acpihid;
struct module *reset_module;
struct device *device;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V5 4/6] vfio: platform: add support for ACPI probe
2016-05-16 2:13 ` [PATCH V5 4/6] vfio: platform: add support for ACPI probe Sinan Kaya
@ 2016-05-23 13:18 ` Eric Auger
2016-05-24 2:18 ` Sinan Kaya
2016-06-07 16:33 ` Auger Eric
0 siblings, 2 replies; 19+ messages in thread
From: Eric Auger @ 2016-05-23 13:18 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> The code is using the compatible DT string to associate a reset driver
> with the actual device itself. The compatible string does not exist on
> ACPI based systems. HID is the unique identifier for a device driver
> instead.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 58 ++++++++++++++++++++++++---
> drivers/vfio/platform/vfio_platform_private.h | 1 +
> 2 files changed, 54 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index 25378bd..d859d3b 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -13,6 +13,7 @@
> */
>
> #include <linux/device.h>
> +#include <linux/acpi.h>
> #include <linux/iommu.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> @@ -49,6 +50,37 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
> return reset_fn;
> }
>
> +#ifdef CONFIG_ACPI
> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> + struct device *dev)
should be static
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (acpi_disabled)
> + return -ENODEV;
> +
> + if (!adev) {
> + pr_err("VFIO: ACPI companion device not found for %s\n",
> + vdev->name);
> + return -ENODEV;
> + }
> +
> + vdev->acpihid = acpi_device_hid(adev);
> + if (!vdev->acpihid) {
> + pr_err("VFIO: cannot find ACPI HID for %s\n",
> + vdev->name);
> + return -ENODEV;
> + }
> + return 0;
> +}
> +#else
> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> + struct device *dev)
static inline?
> +{
> + return -EINVAL;
nit: -ENOENT ?
> +}
> +#endif
> +
> static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
> {
> return vdev->of_reset ? true : false;
> @@ -548,6 +580,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
> .mmap = vfio_platform_mmap,
> };
>
> +int vfio_platform_of_probe(struct vfio_platform_device *vdev,
> + struct device *dev)
> +{
> + int ret;
> +
> + ret = device_property_read_string(dev, "compatible",
> + &vdev->compat);
> + if (ret) {
> + pr_err("VFIO: cannot retrieve compat for %s\n",
> + vdev->name);
> + return ret;
> + }
> + return 0;
nit: can be simplified by returning ret in any case.
Best Regards
Eric
> +}
> +
> int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> struct device *dev)
> {
> @@ -557,11 +604,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> if (!vdev)
> return -EINVAL;
>
> - ret = device_property_read_string(dev, "compatible", &vdev->compat);
> - if (ret) {
> - pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
> - return -EINVAL;
> - }
> + ret = vfio_platform_acpi_probe(vdev, dev);
> + if (ret)
> + ret = vfio_platform_of_probe(vdev, dev);
> +
> + if (ret)
> + return ret;
>
> vdev->device = dev;
>
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 71ed7d1..ba9e4f8 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -58,6 +58,7 @@ struct vfio_platform_device {
> struct mutex igate;
> struct module *parent_module;
> const char *compat;
> + const char *acpihid;
> struct module *reset_module;
> struct device *device;
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH V5 4/6] vfio: platform: add support for ACPI probe
2016-05-23 13:18 ` Eric Auger
@ 2016-05-24 2:18 ` Sinan Kaya
2016-06-07 16:33 ` Auger Eric
1 sibling, 0 replies; 19+ messages in thread
From: Sinan Kaya @ 2016-05-24 2:18 UTC (permalink / raw)
To: linux-arm-kernel
On 5/23/2016 9:18 AM, Eric Auger wrote:
> On 05/16/2016 04:13 AM, Sinan Kaya wrote:
>> The code is using the compatible DT string to associate a reset driver
>> with the actual device itself. The compatible string does not exist on
>> ACPI based systems. HID is the unique identifier for a device driver
>> instead.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 58 ++++++++++++++++++++++++---
>> drivers/vfio/platform/vfio_platform_private.h | 1 +
>> 2 files changed, 54 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index 25378bd..d859d3b 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -13,6 +13,7 @@
>> */
>>
>> #include <linux/device.h>
>> +#include <linux/acpi.h>
>> #include <linux/iommu.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> @@ -49,6 +50,37 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
>> return reset_fn;
>> }
>>
>> +#ifdef CONFIG_ACPI
>> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
> should be static
makes sense
>> +{
>> + struct acpi_device *adev = ACPI_COMPANION(dev);
>> +
>> + if (acpi_disabled)
>> + return -ENODEV;
>> +
>> + if (!adev) {
>> + pr_err("VFIO: ACPI companion device not found for %s\n",
>> + vdev->name);
>> + return -ENODEV;
>> + }
>> +
>> + vdev->acpihid = acpi_device_hid(adev);
>> + if (!vdev->acpihid) {
>> + pr_err("VFIO: cannot find ACPI HID for %s\n",
>> + vdev->name);
>> + return -ENODEV;
>> + }
>> + return 0;
>> +}
>> +#else
>> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
> static inline?
yup
>> +{
>> + return -EINVAL;
> nit: -ENOENT ?
OK
>> +}
>> +#endif
>> +
>> static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
>> {
>> return vdev->of_reset ? true : false;
>> @@ -548,6 +580,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
>> .mmap = vfio_platform_mmap,
>> };
>>
>> +int vfio_platform_of_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
>> +{
>> + int ret;
>> +
>> + ret = device_property_read_string(dev, "compatible",
>> + &vdev->compat);
>> + if (ret) {
>> + pr_err("VFIO: cannot retrieve compat for %s\n",
>> + vdev->name);
>> + return ret;
>> + }
>> + return 0;
> nit: can be simplified by returning ret in any case.
will do
>
> Best Regards
>
> Eric
>
>> +}
>> +
>> int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> struct device *dev)
>> {
>> @@ -557,11 +604,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> if (!vdev)
>> return -EINVAL;
>>
>> - ret = device_property_read_string(dev, "compatible", &vdev->compat);
>> - if (ret) {
>> - pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
>> - return -EINVAL;
>> - }
>> + ret = vfio_platform_acpi_probe(vdev, dev);
>> + if (ret)
>> + ret = vfio_platform_of_probe(vdev, dev);
>> +
>> + if (ret)
>> + return ret;
>>
>> vdev->device = dev;
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
>> index 71ed7d1..ba9e4f8 100644
>> --- a/drivers/vfio/platform/vfio_platform_private.h
>> +++ b/drivers/vfio/platform/vfio_platform_private.h
>> @@ -58,6 +58,7 @@ struct vfio_platform_device {
>> struct mutex igate;
>> struct module *parent_module;
>> const char *compat;
>> + const char *acpihid;
>> struct module *reset_module;
>> struct device *device;
>>
>>
>
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH V5 4/6] vfio: platform: add support for ACPI probe
2016-05-23 13:18 ` Eric Auger
2016-05-24 2:18 ` Sinan Kaya
@ 2016-06-07 16:33 ` Auger Eric
1 sibling, 0 replies; 19+ messages in thread
From: Auger Eric @ 2016-06-07 16:33 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sinan,
Le 23/05/2016 ? 15:18, Eric Auger a ?crit :
> On 05/16/2016 04:13 AM, Sinan Kaya wrote:
>> The code is using the compatible DT string to associate a reset driver
>> with the actual device itself. The compatible string does not exist on
>> ACPI based systems. HID is the unique identifier for a device driver
>> instead.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 58 ++++++++++++++++++++++++---
>> drivers/vfio/platform/vfio_platform_private.h | 1 +
>> 2 files changed, 54 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index 25378bd..d859d3b 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -13,6 +13,7 @@
>> */
>>
>> #include <linux/device.h>
>> +#include <linux/acpi.h>
>> #include <linux/iommu.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> @@ -49,6 +50,37 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
>> return reset_fn;
>> }
>>
>> +#ifdef CONFIG_ACPI
>> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
> should be static
>> +{
>> + struct acpi_device *adev = ACPI_COMPANION(dev);
>> +
>> + if (acpi_disabled)
>> + return -ENODEV;
>> +
>> + if (!adev) {
>> + pr_err("VFIO: ACPI companion device not found for %s\n",
>> + vdev->name);
>> + return -ENODEV;
>> + }
>> +
>> + vdev->acpihid = acpi_device_hid(adev);
>> + if (!vdev->acpihid) {
>> + pr_err("VFIO: cannot find ACPI HID for %s\n",
>> + vdev->name);
>> + return -ENODEV;
>> + }
>> + return 0;
>> +}
>> +#else
>> +int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
> static inline?
>> +{
>> + return -EINVAL;
> nit: -ENOENT ?
>> +}
>> +#endif
>> +
>> static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
>> {
>> return vdev->of_reset ? true : false;
>> @@ -548,6 +580,21 @@ static const struct vfio_device_ops vfio_platform_ops = {
>> .mmap = vfio_platform_mmap,
>> };
>>
>> +int vfio_platform_of_probe(struct vfio_platform_device *vdev,
>> + struct device *dev)
>> +{
>> + int ret;
>> +
>> + ret = device_property_read_string(dev, "compatible",
>> + &vdev->compat);
>> + if (ret) {
>> + pr_err("VFIO: cannot retrieve compat for %s\n",
>> + vdev->name);
>> + return ret;
>> + }
>> + return 0;
> nit: can be simplified by returning ret in any case.
>
> Best Regards
>
> Eric
>
>> +}
>> +
>> int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> struct device *dev)
>> {
>> @@ -557,11 +604,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>> if (!vdev)
>> return -EINVAL;
>>
>> - ret = device_property_read_string(dev, "compatible", &vdev->compat);
>> - if (ret) {
>> - pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
>> - return -EINVAL;
>> - }
>> + ret = vfio_platform_acpi_probe(vdev, dev);
>> + if (ret)
>> + ret = vfio_platform_of_probe(vdev, dev);
>> +
>> + if (ret)
>> + return ret;
>>
>> vdev->device = dev;
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
>> index 71ed7d1..ba9e4f8 100644
>> --- a/drivers/vfio/platform/vfio_platform_private.h
>> +++ b/drivers/vfio/platform/vfio_platform_private.h
>> @@ -58,6 +58,7 @@ struct vfio_platform_device {
>> struct mutex igate;
>> struct module *parent_module;
>> const char *compat;
>> + const char *acpihid;
>> struct module *reset_module;
>> struct device *device;
>>
>>
>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Best Regards
Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI
2016-05-16 2:13 [PATCH V5 0/6] vfio, platform: add ACPI support Sinan Kaya
` (3 preceding siblings ...)
2016-05-16 2:13 ` [PATCH V5 4/6] vfio: platform: add support for ACPI probe Sinan Kaya
@ 2016-05-16 2:13 ` Sinan Kaya
2016-05-23 14:41 ` Eric Auger
2016-05-23 15:21 ` Eric Auger
2016-05-16 2:13 ` [PATCH V5 6/6] vfio, platform: make reset driver a requirement by default Sinan Kaya
5 siblings, 2 replies; 19+ messages in thread
From: Sinan Kaya @ 2016-05-16 2:13 UTC (permalink / raw)
To: linux-arm-kernel
The device tree code checks for the presence of a reset driver and calls
the of_reset function pointer by looking up the reset driver as a module.
ACPI defines _RST method to perform device level reset. After the _RST
method is executed, the OS can resume using the device.
This patch checks the presence of _RST method and calls the _RST
method when reset is requested.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 44 ++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index d859d3b..095d5b7 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -73,21 +73,59 @@ int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
}
return 0;
}
+
+static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
+{
+ struct device *dev = vdev->device;
+ acpi_handle handle = ACPI_HANDLE(dev);
+ acpi_status acpi_ret;
+ unsigned long long val;
+
+ acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
+ if (ACPI_FAILURE(acpi_ret))
+ return -EINVAL;
+
+ return 0;
+}
+
+static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
+{
+ struct device *dev = vdev->device;
+ acpi_handle handle = ACPI_HANDLE(dev);
+
+ return acpi_has_method(handle, "_RST");
+}
#else
int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
struct device *dev)
{
return -EINVAL;
}
+
+static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
+{
+ return -EINVAL;
+}
+
+static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
+{
+ return false;
+}
#endif
static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
{
+ if (vdev->acpihid)
+ return vfio_platform_acpi_has_reset(vdev);
+
return vdev->of_reset ? true : false;
}
static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
{
+ if (vdev->acpihid)
+ return;
+
vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
&vdev->reset_module);
if (!vdev->of_reset) {
@@ -99,6 +137,9 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
{
+ if (vdev->acpihid)
+ return;
+
if (vdev->of_reset)
module_put(vdev->reset_module);
}
@@ -177,6 +218,9 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
dev_info(vdev->device, "reset\n");
vdev->of_reset(vdev);
return 0;
+ } else if (vdev->acpihid) {
+ dev_info(vdev->device, "reset\n");
+ return vfio_platform_acpi_call_reset(vdev);
}
dev_warn(vdev->device, "no reset function found!\n");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI
2016-05-16 2:13 ` [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI Sinan Kaya
@ 2016-05-23 14:41 ` Eric Auger
2016-05-24 2:23 ` Sinan Kaya
2016-05-23 15:21 ` Eric Auger
1 sibling, 1 reply; 19+ messages in thread
From: Eric Auger @ 2016-05-23 14:41 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> The device tree code checks for the presence of a reset driver and calls
> the of_reset function pointer by looking up the reset driver as a module.
>
> ACPI defines _RST method to perform device level reset. After the _RST
> method is executed, the OS can resume using the device.
>
> This patch checks the presence of _RST method and calls the _RST
> method when reset is requested.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 44 ++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index d859d3b..095d5b7 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -73,21 +73,59 @@ int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> }
> return 0;
> }
> +
> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
> +{
> + struct device *dev = vdev->device;
> + acpi_handle handle = ACPI_HANDLE(dev);
> + acpi_status acpi_ret;
> + unsigned long long val;
> +
> + acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
> + if (ACPI_FAILURE(acpi_ret))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
> +{
> + struct device *dev = vdev->device;
> + acpi_handle handle = ACPI_HANDLE(dev);
> +
> + return acpi_has_method(handle, "_RST");
> +}
> #else
> int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> struct device *dev)
> {
> return -EINVAL;
> }
> +
> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
> +{
nit: inline
> + return -EINVAL;
> +}
> +
> +static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
nit: inline
Besides Reviewed-by: Eric Auger <eric.auger@linaro.org>
It is not explictly written in the ACPI spec that _RST is expected to
stop DMA transfers and IRQs although this may be implicit in the notion
of "function level reset ". May be worth adding this info in the commit msg?
Best Regards
Eric
> +{
> + return false;
> +}
> #endif
>
> static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
> {
> + if (vdev->acpihid)
> + return vfio_platform_acpi_has_reset(vdev);
> +
> return vdev->of_reset ? true : false;
> }
>
> static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> {
> + if (vdev->acpihid)
> + return;
> +
> vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> &vdev->reset_module);
> if (!vdev->of_reset) {
> @@ -99,6 +137,9 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
>
> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> {
> + if (vdev->acpihid)
> + return;
> +
> if (vdev->of_reset)
> module_put(vdev->reset_module);
> }
> @@ -177,6 +218,9 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
> dev_info(vdev->device, "reset\n");
> vdev->of_reset(vdev);
> return 0;
> + } else if (vdev->acpihid) {
> + dev_info(vdev->device, "reset\n");
> + return vfio_platform_acpi_call_reset(vdev);
> }
>
> dev_warn(vdev->device, "no reset function found!\n");
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI
2016-05-23 14:41 ` Eric Auger
@ 2016-05-24 2:23 ` Sinan Kaya
0 siblings, 0 replies; 19+ messages in thread
From: Sinan Kaya @ 2016-05-24 2:23 UTC (permalink / raw)
To: linux-arm-kernel
On 5/23/2016 10:41 AM, Eric Auger wrote:
> On 05/16/2016 04:13 AM, Sinan Kaya wrote:
>> The device tree code checks for the presence of a reset driver and calls
>> the of_reset function pointer by looking up the reset driver as a module.
>>
>> ACPI defines _RST method to perform device level reset. After the _RST
>> method is executed, the OS can resume using the device.
>>
>> This patch checks the presence of _RST method and calls the _RST
>> method when reset is requested.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 44 ++++++++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index d859d3b..095d5b7 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -73,21 +73,59 @@ int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> }
>> return 0;
>> }
>> +
>> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
>> +{
>> + struct device *dev = vdev->device;
>> + acpi_handle handle = ACPI_HANDLE(dev);
>> + acpi_status acpi_ret;
>> + unsigned long long val;
>> +
>> + acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
>> + if (ACPI_FAILURE(acpi_ret))
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
>> +{
>> + struct device *dev = vdev->device;
>> + acpi_handle handle = ACPI_HANDLE(dev);
>> +
>> + return acpi_has_method(handle, "_RST");
>> +}
>> #else
>> int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
>> struct device *dev)
>> {
>> return -EINVAL;
>> }
>> +
>> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
>> +{
> nit: inline
>> + return -EINVAL;
>> +}
>> +
>> +static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
> nit: inline
sure
>
> Besides Reviewed-by: Eric Auger <eric.auger@linaro.org>
>
thanks
> It is not explictly written in the ACPI spec that _RST is expected to
> stop DMA transfers and IRQs although this may be implicit in the notion
> of "function level reset ". May be worth adding this info in the commit msg?
>
yes, I'll add DMA and IRQ requirement to the commit message.
> Best Regards
>
> Eric
>> +{
>> + return false;
>> +}
>> #endif
>>
>> static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
>> {
>> + if (vdev->acpihid)
>> + return vfio_platform_acpi_has_reset(vdev);
>> +
>> return vdev->of_reset ? true : false;
>> }
>>
>> static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
>> {
>> + if (vdev->acpihid)
>> + return;
>> +
>> vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
>> &vdev->reset_module);
>> if (!vdev->of_reset) {
>> @@ -99,6 +137,9 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
>>
>> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
>> {
>> + if (vdev->acpihid)
>> + return;
>> +
>> if (vdev->of_reset)
>> module_put(vdev->reset_module);
>> }
>> @@ -177,6 +218,9 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
>> dev_info(vdev->device, "reset\n");
>> vdev->of_reset(vdev);
>> return 0;
>> + } else if (vdev->acpihid) {
>> + dev_info(vdev->device, "reset\n");
>> + return vfio_platform_acpi_call_reset(vdev);
>> }
>>
>> dev_warn(vdev->device, "no reset function found!\n");
>>
>
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI
2016-05-16 2:13 ` [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI Sinan Kaya
2016-05-23 14:41 ` Eric Auger
@ 2016-05-23 15:21 ` Eric Auger
2016-05-24 2:25 ` Sinan Kaya
1 sibling, 1 reply; 19+ messages in thread
From: Eric Auger @ 2016-05-23 15:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sinan,
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> The device tree code checks for the presence of a reset driver and calls
> the of_reset function pointer by looking up the reset driver as a module.
>
> ACPI defines _RST method to perform device level reset. After the _RST
> method is executed, the OS can resume using the device.
>
> This patch checks the presence of _RST method and calls the _RST
> method when reset is requested.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 44 ++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index d859d3b..095d5b7 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -73,21 +73,59 @@ int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> }
> return 0;
> }
> +
> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
> +{
> + struct device *dev = vdev->device;
> + acpi_handle handle = ACPI_HANDLE(dev);
> + acpi_status acpi_ret;
> + unsigned long long val;
> +
> + acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
> + if (ACPI_FAILURE(acpi_ret))
> + return -EINVAL;
Can't you return something more explicit here? The error code will be
visible to the userspace. Difficult for him to understand the cause of
the failure in that case.
Eric
> +
> + return 0;
> +}
> +
> +static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
> +{
> + struct device *dev = vdev->device;
> + acpi_handle handle = ACPI_HANDLE(dev);
> +
> + return acpi_has_method(handle, "_RST");
> +}
> #else
> int vfio_platform_acpi_probe(struct vfio_platform_device *vdev,
> struct device *dev)
> {
> return -EINVAL;
> }
> +
> +static int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev)
> +{
> + return -EINVAL;
> +}
> +
> +static bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev)
> +{
> + return false;
> +}
> #endif
>
> static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
> {
> + if (vdev->acpihid)
> + return vfio_platform_acpi_has_reset(vdev);
> +
> return vdev->of_reset ? true : false;
> }
>
> static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> {
> + if (vdev->acpihid)
> + return;
> +
> vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> &vdev->reset_module);
> if (!vdev->of_reset) {
> @@ -99,6 +137,9 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
>
> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> {
> + if (vdev->acpihid)
> + return;
> +
> if (vdev->of_reset)
> module_put(vdev->reset_module);
> }
> @@ -177,6 +218,9 @@ static int vfio_platform_call_reset(struct vfio_platform_device *vdev)
> dev_info(vdev->device, "reset\n");
> vdev->of_reset(vdev);
> return 0;
> + } else if (vdev->acpihid) {
> + dev_info(vdev->device, "reset\n");
> + return vfio_platform_acpi_call_reset(vdev);
> }
>
> dev_warn(vdev->device, "no reset function found!\n");
>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI
2016-05-23 15:21 ` Eric Auger
@ 2016-05-24 2:25 ` Sinan Kaya
0 siblings, 0 replies; 19+ messages in thread
From: Sinan Kaya @ 2016-05-24 2:25 UTC (permalink / raw)
To: linux-arm-kernel
On 5/23/2016 11:21 AM, Eric Auger wrote:
>> + acpi_ret = acpi_evaluate_integer(handle, "_RST", NULL, &val);
>> > + if (ACPI_FAILURE(acpi_ret))
>> > + return -EINVAL;
> Can't you return something more explicit here? The error code will be
> visible to the userspace. Difficult for him to understand the cause of
> the failure in that case.
I can also add a reset failed message to both DT and ACPI reset calls.
Will that work?
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V5 6/6] vfio, platform: make reset driver a requirement by default
2016-05-16 2:13 [PATCH V5 0/6] vfio, platform: add ACPI support Sinan Kaya
` (4 preceding siblings ...)
2016-05-16 2:13 ` [PATCH V5 5/6] vfio: platform: call _RST method when using ACPI Sinan Kaya
@ 2016-05-16 2:13 ` Sinan Kaya
2016-05-23 15:20 ` Eric Auger
5 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-05-16 2:13 UTC (permalink / raw)
To: linux-arm-kernel
The code was allowing platform devices to be used without a supporting
VFIO reset driver. The hardware can be left in some inconsistent state
after a guest machine abort.
The reset driver will put the hardware back to safe state and disable
interrupts before returning the control back to the host machine.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_amba.c | 5 +++++
drivers/vfio/platform/vfio_platform.c | 5 +++++
drivers/vfio/platform/vfio_platform_common.c | 18 ++++++++++++++----
drivers/vfio/platform/vfio_platform_private.h | 1 +
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
index a66479b..7585902 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -23,6 +23,10 @@
#define DRIVER_AUTHOR "Antonios Motakis <a.motakis@virtualopensystems.com>"
#define DRIVER_DESC "VFIO for AMBA devices - User Level meta-driver"
+static bool reset_required = true;
+module_param(reset_required, bool, 0644);
+MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)");
+
/* probing devices from the AMBA bus */
static struct resource *get_amba_resource(struct vfio_platform_device *vdev,
@@ -68,6 +72,7 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id)
vdev->get_resource = get_amba_resource;
vdev->get_irq = get_amba_irq;
vdev->parent_module = THIS_MODULE;
+ vdev->reset_required = reset_required;
ret = vfio_platform_probe_common(vdev, &adev->dev);
if (ret) {
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index b1cc3a7..ef89146 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -23,6 +23,10 @@
#define DRIVER_AUTHOR "Antonios Motakis <a.motakis@virtualopensystems.com>"
#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver"
+static bool reset_required = true;
+module_param(reset_required, bool, 0644);
+MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)");
+
/* probing devices from the linux platform bus */
static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
@@ -66,6 +70,7 @@ static int vfio_platform_probe(struct platform_device *pdev)
vdev->get_resource = get_platform_resource;
vdev->get_irq = get_platform_irq;
vdev->parent_module = THIS_MODULE;
+ vdev->reset_required = reset_required;
ret = vfio_platform_probe_common(vdev, &pdev->dev);
if (ret)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 095d5b7..89fb18f 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -121,10 +121,10 @@ static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
return vdev->of_reset ? true : false;
}
-static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
+static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
{
if (vdev->acpihid)
- return;
+ return vfio_platform_acpi_has_reset(vdev) ? 0 : -EINVAL;
vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
&vdev->reset_module);
@@ -133,6 +133,8 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
&vdev->reset_module);
}
+
+ return vdev->of_reset ? 0 : -EINVAL;
}
static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
@@ -263,7 +265,9 @@ static int vfio_platform_open(void *device_data)
if (ret)
goto err_irq;
- vfio_platform_call_reset(vdev);
+ ret = vfio_platform_call_reset(vdev);
+ if (ret && vdev->reset_required)
+ goto err_irq;
}
vdev->refcnt++;
@@ -669,7 +673,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
return ret;
}
- vfio_platform_get_reset(vdev);
+ ret = vfio_platform_get_reset(vdev);
+ if (ret && vdev->reset_required) {
+ pr_err("vfio: no reset function found for device %s\n",
+ vdev->name);
+ iommu_group_put(group);
+ return ret;
+ }
mutex_init(&vdev->igate);
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index ba9e4f8..68fbc00 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -50,6 +50,7 @@ struct vfio_platform_region {
};
struct vfio_platform_device {
+ bool reset_required;
struct vfio_platform_region *regions;
u32 num_regions;
struct vfio_platform_irq *irqs;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V5 6/6] vfio, platform: make reset driver a requirement by default
2016-05-16 2:13 ` [PATCH V5 6/6] vfio, platform: make reset driver a requirement by default Sinan Kaya
@ 2016-05-23 15:20 ` Eric Auger
0 siblings, 0 replies; 19+ messages in thread
From: Eric Auger @ 2016-05-23 15:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sinan,
On 05/16/2016 04:13 AM, Sinan Kaya wrote:
> The code was allowing platform devices to be used without a supporting
> VFIO reset driver. The hardware can be left in some inconsistent state
> after a guest machine abort.
>
> The reset driver will put the hardware back to safe state and disable
> interrupts before returning the control back to the host machine.
The commit message should describe the new module option.
You should also describe this is not just a matter of having a reset
function implemented & found somewhere but also a matter of having the
reset call to succeed. A reset failure now induce a failure on the first
open(). Shouldn't we handle this failure in a separate patch to make
this clearer?
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_amba.c | 5 +++++
> drivers/vfio/platform/vfio_platform.c | 5 +++++
> drivers/vfio/platform/vfio_platform_common.c | 18 ++++++++++++++----
> drivers/vfio/platform/vfio_platform_private.h | 1 +
> 4 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
> index a66479b..7585902 100644
> --- a/drivers/vfio/platform/vfio_amba.c
> +++ b/drivers/vfio/platform/vfio_amba.c
> @@ -23,6 +23,10 @@
> #define DRIVER_AUTHOR "Antonios Motakis <a.motakis@virtualopensystems.com>"
> #define DRIVER_DESC "VFIO for AMBA devices - User Level meta-driver"
>
> +static bool reset_required = true;
> +module_param(reset_required, bool, 0644);
> +MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)");
> +
> /* probing devices from the AMBA bus */
>
> static struct resource *get_amba_resource(struct vfio_platform_device *vdev,
> @@ -68,6 +72,7 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id)
> vdev->get_resource = get_amba_resource;
> vdev->get_irq = get_amba_irq;
> vdev->parent_module = THIS_MODULE;
> + vdev->reset_required = reset_required;
>
> ret = vfio_platform_probe_common(vdev, &adev->dev);
> if (ret) {
> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> index b1cc3a7..ef89146 100644
> --- a/drivers/vfio/platform/vfio_platform.c
> +++ b/drivers/vfio/platform/vfio_platform.c
> @@ -23,6 +23,10 @@
> #define DRIVER_AUTHOR "Antonios Motakis <a.motakis@virtualopensystems.com>"
> #define DRIVER_DESC "VFIO for platform devices - User Level meta-driver"
>
> +static bool reset_required = true;
> +module_param(reset_required, bool, 0644);
> +MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)");
> +
> /* probing devices from the linux platform bus */
>
> static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> @@ -66,6 +70,7 @@ static int vfio_platform_probe(struct platform_device *pdev)
> vdev->get_resource = get_platform_resource;
> vdev->get_irq = get_platform_irq;
> vdev->parent_module = THIS_MODULE;
> + vdev->reset_required = reset_required;
>
> ret = vfio_platform_probe_common(vdev, &pdev->dev);
> if (ret)
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index 095d5b7..89fb18f 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -121,10 +121,10 @@ static bool vfio_platform_has_reset(struct vfio_platform_device *vdev)
> return vdev->of_reset ? true : false;
> }
>
> -static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> +static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
> {
> if (vdev->acpihid)
> - return;
> + return vfio_platform_acpi_has_reset(vdev) ? 0 : -EINVAL;
-ENOENT instead?
>
> vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> &vdev->reset_module);
> @@ -133,6 +133,8 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
> vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> &vdev->reset_module);
> }
> +
> + return vdev->of_reset ? 0 : -EINVAL;
same
> }
>
> static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> @@ -263,7 +265,9 @@ static int vfio_platform_open(void *device_data)
> if (ret)
> goto err_irq;
>
> - vfio_platform_call_reset(vdev);
> + ret = vfio_platform_call_reset(vdev);
> + if (ret && vdev->reset_required)
> + goto err_irq;
what do we do at release time in case the reset fails. You did not
change anything. Shouldn't we at least emit a warning to the user if the
reset becomes mandated (separate patch as evoked above)?
Thanks
Eric
> }
>
> vdev->refcnt++;
> @@ -669,7 +673,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> return ret;
> }
>
> - vfio_platform_get_reset(vdev);
> + ret = vfio_platform_get_reset(vdev);
> + if (ret && vdev->reset_required) {
> + pr_err("vfio: no reset function found for device %s\n",
> + vdev->name);
> + iommu_group_put(group);
> + return ret;
> + }
>
> mutex_init(&vdev->igate);
>
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index ba9e4f8..68fbc00 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -50,6 +50,7 @@ struct vfio_platform_region {
> };
>
> struct vfio_platform_device {
> + bool reset_required;
> struct vfio_platform_region *regions;
> u32 num_regions;
> struct vfio_platform_irq *irqs;
>
^ permalink raw reply [flat|nested] 19+ messages in thread