Re: [RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices

2021-01-20 Thread Auger Eric
Hi Alex,

On 1/19/21 11:45 PM, Alex Williamson wrote:
> On Fri, 15 Jan 2021 10:24:33 +0100
> Auger Eric  wrote:
> 
>> Hi Vikas,
>> On 1/15/21 7:35 AM, Vikas Gupta wrote:
>>> Hi Eric,
>>>
>>> On Tue, Jan 12, 2021 at 2:52 PM Auger Eric  wrote:  

 Hi Vikas,

 On 12/14/20 6:45 PM, Vikas Gupta wrote:  
> Add msi support for Broadcom platform devices
>
> Signed-off-by: Vikas Gupta 
> ---
>  drivers/vfio/platform/Kconfig |  1 +
>  drivers/vfio/platform/Makefile|  1 +
>  drivers/vfio/platform/msi/Kconfig |  9 
>  drivers/vfio/platform/msi/Makefile|  2 +
>  .../vfio/platform/msi/vfio_platform_bcmplt.c  | 49 +++
>  5 files changed, 62 insertions(+)
>  create mode 100644 drivers/vfio/platform/msi/Kconfig
>  create mode 100644 drivers/vfio/platform/msi/Makefile
>  create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c  
 what does plt mean?  
>>> This(plt) is a generic name for Broadcom platform devices, which we`ll
>>>  plan to add in this file. Currently we have only one in this file.
>>> Do you think this name does not sound good here?  
>>
>> we have VFIO_PLATFORM_BCMFLEXRM_RESET config which also applied to vfio
>> flex-rm platform device.
>>
>> I think it would be more homegenous to have VFIO_PLATFORM_BCMFLEXRM_MSI
>> in case we keep a separate msi module.
>>
>> also in reset dir we have vfio_platform_bcmflexrm.c
>>
>>
>
> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> index dc1a3c44f2c6..7b8696febe61 100644
> --- a/drivers/vfio/platform/Kconfig
> +++ b/drivers/vfio/platform/Kconfig
> @@ -21,3 +21,4 @@ config VFIO_AMBA
> If you don't know what to do here, say N.
>
>  source "drivers/vfio/platform/reset/Kconfig"
> +source "drivers/vfio/platform/msi/Kconfig"
> diff --git a/drivers/vfio/platform/Makefile 
> b/drivers/vfio/platform/Makefile
> index 3f3a24e7c4ef..9ccdcdbf0e7e 100644
> --- a/drivers/vfio/platform/Makefile
> +++ b/drivers/vfio/platform/Makefile
> @@ -5,6 +5,7 @@ vfio-platform-y := vfio_platform.o
>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
>  obj-$(CONFIG_VFIO_PLATFORM) += reset/
> +obj-$(CONFIG_VFIO_PLATFORM) += msi/
>
>  vfio-amba-y := vfio_amba.o
>
> diff --git a/drivers/vfio/platform/msi/Kconfig 
> b/drivers/vfio/platform/msi/Kconfig
> new file mode 100644
> index ..54d6b70e1e32
> --- /dev/null
> +++ b/drivers/vfio/platform/msi/Kconfig
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config VFIO_PLATFORM_BCMPLT_MSI
> + tristate "MSI support for Broadcom platform devices"
> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> + default ARCH_BCM_IPROC
> + help
> +   Enables the VFIO platform driver to handle msi for Broadcom 
> devices
> +
> +   If you don't know what to do here, say N.
> diff --git a/drivers/vfio/platform/msi/Makefile 
> b/drivers/vfio/platform/msi/Makefile
> new file mode 100644
> index ..27422d45cecb
> --- /dev/null
> +++ b/drivers/vfio/platform/msi/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_VFIO_PLATFORM_BCMPLT_MSI) += vfio_platform_bcmplt.o
> diff --git a/drivers/vfio/platform/msi/vfio_platform_bcmplt.c 
> b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> new file mode 100644
> index ..a074b5e92d77
> --- /dev/null
> +++ b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 Broadcom.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../vfio_platform_private.h"
> +
> +#define RING_SIZE(64 << 10)
> +
> +#define RING_MSI_ADDR_LS 0x03c
> +#define RING_MSI_ADDR_MS 0x040
> +#define RING_MSI_DATA_VALUE  0x064  
 Those 3 defines would not be needed anymore with that implementation 
 option.  
> +
> +static u32 bcm_num_msi(struct vfio_platform_device *vdev)
> +{
> + struct vfio_platform_region *reg = >regions[0];
> +
> + return (reg->size / RING_SIZE);
> +}
> +
> +static struct vfio_platform_msi_node vfio_platform_bcmflexrm_msi_node = {
> + .owner = THIS_MODULE,
> + .compat = "brcm,iproc-flexrm-mbox",
> + .of_get_msi = bcm_num_msi,
> +};
> +
> +static int __init vfio_platform_bcmflexrm_msi_module_init(void)
> +{
> + __vfio_platform_register_msi(_platform_bcmflexrm_msi_node);
> +
> + return 0;
> +}
> +
> +static void __exit 

Re: [RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices

2021-01-19 Thread Alex Williamson
On Fri, 15 Jan 2021 10:24:33 +0100
Auger Eric  wrote:

> Hi Vikas,
> On 1/15/21 7:35 AM, Vikas Gupta wrote:
> > Hi Eric,
> > 
> > On Tue, Jan 12, 2021 at 2:52 PM Auger Eric  wrote:  
> >>
> >> Hi Vikas,
> >>
> >> On 12/14/20 6:45 PM, Vikas Gupta wrote:  
> >>> Add msi support for Broadcom platform devices
> >>>
> >>> Signed-off-by: Vikas Gupta 
> >>> ---
> >>>  drivers/vfio/platform/Kconfig |  1 +
> >>>  drivers/vfio/platform/Makefile|  1 +
> >>>  drivers/vfio/platform/msi/Kconfig |  9 
> >>>  drivers/vfio/platform/msi/Makefile|  2 +
> >>>  .../vfio/platform/msi/vfio_platform_bcmplt.c  | 49 +++
> >>>  5 files changed, 62 insertions(+)
> >>>  create mode 100644 drivers/vfio/platform/msi/Kconfig
> >>>  create mode 100644 drivers/vfio/platform/msi/Makefile
> >>>  create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c  
> >> what does plt mean?  
> > This(plt) is a generic name for Broadcom platform devices, which we`ll
> >  plan to add in this file. Currently we have only one in this file.
> > Do you think this name does not sound good here?  
> 
> we have VFIO_PLATFORM_BCMFLEXRM_RESET config which also applied to vfio
> flex-rm platform device.
> 
> I think it would be more homegenous to have VFIO_PLATFORM_BCMFLEXRM_MSI
> in case we keep a separate msi module.
> 
> also in reset dir we have vfio_platform_bcmflexrm.c
> 
> 
> >>>
> >>> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> >>> index dc1a3c44f2c6..7b8696febe61 100644
> >>> --- a/drivers/vfio/platform/Kconfig
> >>> +++ b/drivers/vfio/platform/Kconfig
> >>> @@ -21,3 +21,4 @@ config VFIO_AMBA
> >>> If you don't know what to do here, say N.
> >>>
> >>>  source "drivers/vfio/platform/reset/Kconfig"
> >>> +source "drivers/vfio/platform/msi/Kconfig"
> >>> diff --git a/drivers/vfio/platform/Makefile 
> >>> b/drivers/vfio/platform/Makefile
> >>> index 3f3a24e7c4ef..9ccdcdbf0e7e 100644
> >>> --- a/drivers/vfio/platform/Makefile
> >>> +++ b/drivers/vfio/platform/Makefile
> >>> @@ -5,6 +5,7 @@ vfio-platform-y := vfio_platform.o
> >>>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
> >>>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
> >>>  obj-$(CONFIG_VFIO_PLATFORM) += reset/
> >>> +obj-$(CONFIG_VFIO_PLATFORM) += msi/
> >>>
> >>>  vfio-amba-y := vfio_amba.o
> >>>
> >>> diff --git a/drivers/vfio/platform/msi/Kconfig 
> >>> b/drivers/vfio/platform/msi/Kconfig
> >>> new file mode 100644
> >>> index ..54d6b70e1e32
> >>> --- /dev/null
> >>> +++ b/drivers/vfio/platform/msi/Kconfig
> >>> @@ -0,0 +1,9 @@
> >>> +# SPDX-License-Identifier: GPL-2.0-only
> >>> +config VFIO_PLATFORM_BCMPLT_MSI
> >>> + tristate "MSI support for Broadcom platform devices"
> >>> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> >>> + default ARCH_BCM_IPROC
> >>> + help
> >>> +   Enables the VFIO platform driver to handle msi for Broadcom 
> >>> devices
> >>> +
> >>> +   If you don't know what to do here, say N.
> >>> diff --git a/drivers/vfio/platform/msi/Makefile 
> >>> b/drivers/vfio/platform/msi/Makefile
> >>> new file mode 100644
> >>> index ..27422d45cecb
> >>> --- /dev/null
> >>> +++ b/drivers/vfio/platform/msi/Makefile
> >>> @@ -0,0 +1,2 @@
> >>> +# SPDX-License-Identifier: GPL-2.0
> >>> +obj-$(CONFIG_VFIO_PLATFORM_BCMPLT_MSI) += vfio_platform_bcmplt.o
> >>> diff --git a/drivers/vfio/platform/msi/vfio_platform_bcmplt.c 
> >>> b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> >>> new file mode 100644
> >>> index ..a074b5e92d77
> >>> --- /dev/null
> >>> +++ b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> >>> @@ -0,0 +1,49 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright 2020 Broadcom.
> >>> + */
> >>> +
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +
> >>> +#include "../vfio_platform_private.h"
> >>> +
> >>> +#define RING_SIZE(64 << 10)
> >>> +
> >>> +#define RING_MSI_ADDR_LS 0x03c
> >>> +#define RING_MSI_ADDR_MS 0x040
> >>> +#define RING_MSI_DATA_VALUE  0x064  
> >> Those 3 defines would not be needed anymore with that implementation 
> >> option.  
> >>> +
> >>> +static u32 bcm_num_msi(struct vfio_platform_device *vdev)
> >>> +{
> >>> + struct vfio_platform_region *reg = >regions[0];
> >>> +
> >>> + return (reg->size / RING_SIZE);
> >>> +}
> >>> +
> >>> +static struct vfio_platform_msi_node vfio_platform_bcmflexrm_msi_node = {
> >>> + .owner = THIS_MODULE,
> >>> + .compat = "brcm,iproc-flexrm-mbox",
> >>> + .of_get_msi = bcm_num_msi,
> >>> +};
> >>> +
> >>> +static int __init vfio_platform_bcmflexrm_msi_module_init(void)
> >>> +{
> >>> + __vfio_platform_register_msi(_platform_bcmflexrm_msi_node);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void __exit vfio_platform_bcmflexrm_msi_module_exit(void)
> >>> +{
> >>> + 

Re: [RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices

2021-01-15 Thread Auger Eric
Hi Vikas,
On 1/15/21 7:35 AM, Vikas Gupta wrote:
> Hi Eric,
> 
> On Tue, Jan 12, 2021 at 2:52 PM Auger Eric  wrote:
>>
>> Hi Vikas,
>>
>> On 12/14/20 6:45 PM, Vikas Gupta wrote:
>>> Add msi support for Broadcom platform devices
>>>
>>> Signed-off-by: Vikas Gupta 
>>> ---
>>>  drivers/vfio/platform/Kconfig |  1 +
>>>  drivers/vfio/platform/Makefile|  1 +
>>>  drivers/vfio/platform/msi/Kconfig |  9 
>>>  drivers/vfio/platform/msi/Makefile|  2 +
>>>  .../vfio/platform/msi/vfio_platform_bcmplt.c  | 49 +++
>>>  5 files changed, 62 insertions(+)
>>>  create mode 100644 drivers/vfio/platform/msi/Kconfig
>>>  create mode 100644 drivers/vfio/platform/msi/Makefile
>>>  create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c
>> what does plt mean?
> This(plt) is a generic name for Broadcom platform devices, which we`ll
>  plan to add in this file. Currently we have only one in this file.
> Do you think this name does not sound good here?

we have VFIO_PLATFORM_BCMFLEXRM_RESET config which also applied to vfio
flex-rm platform device.

I think it would be more homegenous to have VFIO_PLATFORM_BCMFLEXRM_MSI
in case we keep a separate msi module.

also in reset dir we have vfio_platform_bcmflexrm.c


>>>
>>> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
>>> index dc1a3c44f2c6..7b8696febe61 100644
>>> --- a/drivers/vfio/platform/Kconfig
>>> +++ b/drivers/vfio/platform/Kconfig
>>> @@ -21,3 +21,4 @@ config VFIO_AMBA
>>> If you don't know what to do here, say N.
>>>
>>>  source "drivers/vfio/platform/reset/Kconfig"
>>> +source "drivers/vfio/platform/msi/Kconfig"
>>> diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
>>> index 3f3a24e7c4ef..9ccdcdbf0e7e 100644
>>> --- a/drivers/vfio/platform/Makefile
>>> +++ b/drivers/vfio/platform/Makefile
>>> @@ -5,6 +5,7 @@ vfio-platform-y := vfio_platform.o
>>>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
>>>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
>>>  obj-$(CONFIG_VFIO_PLATFORM) += reset/
>>> +obj-$(CONFIG_VFIO_PLATFORM) += msi/
>>>
>>>  vfio-amba-y := vfio_amba.o
>>>
>>> diff --git a/drivers/vfio/platform/msi/Kconfig 
>>> b/drivers/vfio/platform/msi/Kconfig
>>> new file mode 100644
>>> index ..54d6b70e1e32
>>> --- /dev/null
>>> +++ b/drivers/vfio/platform/msi/Kconfig
>>> @@ -0,0 +1,9 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +config VFIO_PLATFORM_BCMPLT_MSI
>>> + tristate "MSI support for Broadcom platform devices"
>>> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>>> + default ARCH_BCM_IPROC
>>> + help
>>> +   Enables the VFIO platform driver to handle msi for Broadcom devices
>>> +
>>> +   If you don't know what to do here, say N.
>>> diff --git a/drivers/vfio/platform/msi/Makefile 
>>> b/drivers/vfio/platform/msi/Makefile
>>> new file mode 100644
>>> index ..27422d45cecb
>>> --- /dev/null
>>> +++ b/drivers/vfio/platform/msi/Makefile
>>> @@ -0,0 +1,2 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +obj-$(CONFIG_VFIO_PLATFORM_BCMPLT_MSI) += vfio_platform_bcmplt.o
>>> diff --git a/drivers/vfio/platform/msi/vfio_platform_bcmplt.c 
>>> b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
>>> new file mode 100644
>>> index ..a074b5e92d77
>>> --- /dev/null
>>> +++ b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
>>> @@ -0,0 +1,49 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright 2020 Broadcom.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include "../vfio_platform_private.h"
>>> +
>>> +#define RING_SIZE(64 << 10)
>>> +
>>> +#define RING_MSI_ADDR_LS 0x03c
>>> +#define RING_MSI_ADDR_MS 0x040
>>> +#define RING_MSI_DATA_VALUE  0x064
>> Those 3 defines would not be needed anymore with that implementation option.
>>> +
>>> +static u32 bcm_num_msi(struct vfio_platform_device *vdev)
>>> +{
>>> + struct vfio_platform_region *reg = >regions[0];
>>> +
>>> + return (reg->size / RING_SIZE);
>>> +}
>>> +
>>> +static struct vfio_platform_msi_node vfio_platform_bcmflexrm_msi_node = {
>>> + .owner = THIS_MODULE,
>>> + .compat = "brcm,iproc-flexrm-mbox",
>>> + .of_get_msi = bcm_num_msi,
>>> +};
>>> +
>>> +static int __init vfio_platform_bcmflexrm_msi_module_init(void)
>>> +{
>>> + __vfio_platform_register_msi(_platform_bcmflexrm_msi_node);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void __exit vfio_platform_bcmflexrm_msi_module_exit(void)
>>> +{
>>> + vfio_platform_unregister_msi("brcm,iproc-flexrm-mbox");
>>> +}
>>> +
>>> +module_init(vfio_platform_bcmflexrm_msi_module_init);
>>> +module_exit(vfio_platform_bcmflexrm_msi_module_exit);
>> One thing I would like to discuss with Alex.
>>
>> As the reset module is mandated (except if reset_required is forced to
>> 0), I am wondering if we shouldn't try to turn the reset module into 

Re: [RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices

2021-01-14 Thread Vikas Gupta
Hi Eric,

On Tue, Jan 12, 2021 at 2:52 PM Auger Eric  wrote:
>
> Hi Vikas,
>
> On 12/14/20 6:45 PM, Vikas Gupta wrote:
> > Add msi support for Broadcom platform devices
> >
> > Signed-off-by: Vikas Gupta 
> > ---
> >  drivers/vfio/platform/Kconfig |  1 +
> >  drivers/vfio/platform/Makefile|  1 +
> >  drivers/vfio/platform/msi/Kconfig |  9 
> >  drivers/vfio/platform/msi/Makefile|  2 +
> >  .../vfio/platform/msi/vfio_platform_bcmplt.c  | 49 +++
> >  5 files changed, 62 insertions(+)
> >  create mode 100644 drivers/vfio/platform/msi/Kconfig
> >  create mode 100644 drivers/vfio/platform/msi/Makefile
> >  create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> what does plt mean?
This(plt) is a generic name for Broadcom platform devices, which we`ll
 plan to add in this file. Currently we have only one in this file.
Do you think this name does not sound good here?
> >
> > diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> > index dc1a3c44f2c6..7b8696febe61 100644
> > --- a/drivers/vfio/platform/Kconfig
> > +++ b/drivers/vfio/platform/Kconfig
> > @@ -21,3 +21,4 @@ config VFIO_AMBA
> > If you don't know what to do here, say N.
> >
> >  source "drivers/vfio/platform/reset/Kconfig"
> > +source "drivers/vfio/platform/msi/Kconfig"
> > diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
> > index 3f3a24e7c4ef..9ccdcdbf0e7e 100644
> > --- a/drivers/vfio/platform/Makefile
> > +++ b/drivers/vfio/platform/Makefile
> > @@ -5,6 +5,7 @@ vfio-platform-y := vfio_platform.o
> >  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
> >  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
> >  obj-$(CONFIG_VFIO_PLATFORM) += reset/
> > +obj-$(CONFIG_VFIO_PLATFORM) += msi/
> >
> >  vfio-amba-y := vfio_amba.o
> >
> > diff --git a/drivers/vfio/platform/msi/Kconfig 
> > b/drivers/vfio/platform/msi/Kconfig
> > new file mode 100644
> > index ..54d6b70e1e32
> > --- /dev/null
> > +++ b/drivers/vfio/platform/msi/Kconfig
> > @@ -0,0 +1,9 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +config VFIO_PLATFORM_BCMPLT_MSI
> > + tristate "MSI support for Broadcom platform devices"
> > + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> > + default ARCH_BCM_IPROC
> > + help
> > +   Enables the VFIO platform driver to handle msi for Broadcom devices
> > +
> > +   If you don't know what to do here, say N.
> > diff --git a/drivers/vfio/platform/msi/Makefile 
> > b/drivers/vfio/platform/msi/Makefile
> > new file mode 100644
> > index ..27422d45cecb
> > --- /dev/null
> > +++ b/drivers/vfio/platform/msi/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_VFIO_PLATFORM_BCMPLT_MSI) += vfio_platform_bcmplt.o
> > diff --git a/drivers/vfio/platform/msi/vfio_platform_bcmplt.c 
> > b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> > new file mode 100644
> > index ..a074b5e92d77
> > --- /dev/null
> > +++ b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> > @@ -0,0 +1,49 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2020 Broadcom.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "../vfio_platform_private.h"
> > +
> > +#define RING_SIZE(64 << 10)
> > +
> > +#define RING_MSI_ADDR_LS 0x03c
> > +#define RING_MSI_ADDR_MS 0x040
> > +#define RING_MSI_DATA_VALUE  0x064
> Those 3 defines would not be needed anymore with that implementation option.
> > +
> > +static u32 bcm_num_msi(struct vfio_platform_device *vdev)
> > +{
> > + struct vfio_platform_region *reg = >regions[0];
> > +
> > + return (reg->size / RING_SIZE);
> > +}
> > +
> > +static struct vfio_platform_msi_node vfio_platform_bcmflexrm_msi_node = {
> > + .owner = THIS_MODULE,
> > + .compat = "brcm,iproc-flexrm-mbox",
> > + .of_get_msi = bcm_num_msi,
> > +};
> > +
> > +static int __init vfio_platform_bcmflexrm_msi_module_init(void)
> > +{
> > + __vfio_platform_register_msi(_platform_bcmflexrm_msi_node);
> > +
> > + return 0;
> > +}
> > +
> > +static void __exit vfio_platform_bcmflexrm_msi_module_exit(void)
> > +{
> > + vfio_platform_unregister_msi("brcm,iproc-flexrm-mbox");
> > +}
> > +
> > +module_init(vfio_platform_bcmflexrm_msi_module_init);
> > +module_exit(vfio_platform_bcmflexrm_msi_module_exit);
> One thing I would like to discuss with Alex.
>
> As the reset module is mandated (except if reset_required is forced to
> 0), I am wondering if we shouldn't try to turn the reset module into a
> "specialization" module and put the msi hooks there. I am afraid we may
> end up having modules for each and every vfio platform feature
> specialization. At the moment that's fully bearable but I can't predict
> what's next.
>
> As the mandated feature is the reset capability maybe we could just keep
> the config/module name 

Re: [RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices

2021-01-12 Thread Auger Eric
Hi Vikas,

On 12/14/20 6:45 PM, Vikas Gupta wrote:
> Add msi support for Broadcom platform devices
> 
> Signed-off-by: Vikas Gupta 
> ---
>  drivers/vfio/platform/Kconfig |  1 +
>  drivers/vfio/platform/Makefile|  1 +
>  drivers/vfio/platform/msi/Kconfig |  9 
>  drivers/vfio/platform/msi/Makefile|  2 +
>  .../vfio/platform/msi/vfio_platform_bcmplt.c  | 49 +++
>  5 files changed, 62 insertions(+)
>  create mode 100644 drivers/vfio/platform/msi/Kconfig
>  create mode 100644 drivers/vfio/platform/msi/Makefile
>  create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c
what does plt mean?
> 
> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> index dc1a3c44f2c6..7b8696febe61 100644
> --- a/drivers/vfio/platform/Kconfig
> +++ b/drivers/vfio/platform/Kconfig
> @@ -21,3 +21,4 @@ config VFIO_AMBA
> If you don't know what to do here, say N.
>  
>  source "drivers/vfio/platform/reset/Kconfig"
> +source "drivers/vfio/platform/msi/Kconfig"
> diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
> index 3f3a24e7c4ef..9ccdcdbf0e7e 100644
> --- a/drivers/vfio/platform/Makefile
> +++ b/drivers/vfio/platform/Makefile
> @@ -5,6 +5,7 @@ vfio-platform-y := vfio_platform.o
>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
>  obj-$(CONFIG_VFIO_PLATFORM) += reset/
> +obj-$(CONFIG_VFIO_PLATFORM) += msi/
>  
>  vfio-amba-y := vfio_amba.o
>  
> diff --git a/drivers/vfio/platform/msi/Kconfig 
> b/drivers/vfio/platform/msi/Kconfig
> new file mode 100644
> index ..54d6b70e1e32
> --- /dev/null
> +++ b/drivers/vfio/platform/msi/Kconfig
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config VFIO_PLATFORM_BCMPLT_MSI
> + tristate "MSI support for Broadcom platform devices"
> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> + default ARCH_BCM_IPROC
> + help
> +   Enables the VFIO platform driver to handle msi for Broadcom devices
> +
> +   If you don't know what to do here, say N.
> diff --git a/drivers/vfio/platform/msi/Makefile 
> b/drivers/vfio/platform/msi/Makefile
> new file mode 100644
> index ..27422d45cecb
> --- /dev/null
> +++ b/drivers/vfio/platform/msi/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_VFIO_PLATFORM_BCMPLT_MSI) += vfio_platform_bcmplt.o
> diff --git a/drivers/vfio/platform/msi/vfio_platform_bcmplt.c 
> b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> new file mode 100644
> index ..a074b5e92d77
> --- /dev/null
> +++ b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 Broadcom.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../vfio_platform_private.h"
> +
> +#define RING_SIZE(64 << 10)
> +
> +#define RING_MSI_ADDR_LS 0x03c
> +#define RING_MSI_ADDR_MS 0x040
> +#define RING_MSI_DATA_VALUE  0x064
Those 3 defines would not be needed anymore with that implementation option.
> +
> +static u32 bcm_num_msi(struct vfio_platform_device *vdev)
> +{
> + struct vfio_platform_region *reg = >regions[0];
> +
> + return (reg->size / RING_SIZE);
> +}
> +
> +static struct vfio_platform_msi_node vfio_platform_bcmflexrm_msi_node = {
> + .owner = THIS_MODULE,
> + .compat = "brcm,iproc-flexrm-mbox",
> + .of_get_msi = bcm_num_msi,
> +};
> +
> +static int __init vfio_platform_bcmflexrm_msi_module_init(void)
> +{
> + __vfio_platform_register_msi(_platform_bcmflexrm_msi_node);
> +
> + return 0;
> +}
> +
> +static void __exit vfio_platform_bcmflexrm_msi_module_exit(void)
> +{
> + vfio_platform_unregister_msi("brcm,iproc-flexrm-mbox");
> +}
> +
> +module_init(vfio_platform_bcmflexrm_msi_module_init);
> +module_exit(vfio_platform_bcmflexrm_msi_module_exit);
One thing I would like to discuss with Alex.

As the reset module is mandated (except if reset_required is forced to
0), I am wondering if we shouldn't try to turn the reset module into a
"specialization" module and put the msi hooks there. I am afraid we may
end up having modules for each and every vfio platform feature
specialization. At the moment that's fully bearable but I can't predict
what's next.

As the mandated feature is the reset capability maybe we could just keep
the config/module name terminology, tune the kconfig help message to
mention the msi support in case of flex-rm?

What do you think?

Thanks

Eric




> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Broadcom");
> 



[RFC v3 2/2] vfio/platform: msi: add Broadcom platform devices

2020-12-14 Thread Vikas Gupta
Add msi support for Broadcom platform devices

Signed-off-by: Vikas Gupta 
---
 drivers/vfio/platform/Kconfig |  1 +
 drivers/vfio/platform/Makefile|  1 +
 drivers/vfio/platform/msi/Kconfig |  9 
 drivers/vfio/platform/msi/Makefile|  2 +
 .../vfio/platform/msi/vfio_platform_bcmplt.c  | 49 +++
 5 files changed, 62 insertions(+)
 create mode 100644 drivers/vfio/platform/msi/Kconfig
 create mode 100644 drivers/vfio/platform/msi/Makefile
 create mode 100644 drivers/vfio/platform/msi/vfio_platform_bcmplt.c

diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
index dc1a3c44f2c6..7b8696febe61 100644
--- a/drivers/vfio/platform/Kconfig
+++ b/drivers/vfio/platform/Kconfig
@@ -21,3 +21,4 @@ config VFIO_AMBA
  If you don't know what to do here, say N.
 
 source "drivers/vfio/platform/reset/Kconfig"
+source "drivers/vfio/platform/msi/Kconfig"
diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 3f3a24e7c4ef..9ccdcdbf0e7e 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -5,6 +5,7 @@ vfio-platform-y := vfio_platform.o
 obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
 obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
 obj-$(CONFIG_VFIO_PLATFORM) += reset/
+obj-$(CONFIG_VFIO_PLATFORM) += msi/
 
 vfio-amba-y := vfio_amba.o
 
diff --git a/drivers/vfio/platform/msi/Kconfig 
b/drivers/vfio/platform/msi/Kconfig
new file mode 100644
index ..54d6b70e1e32
--- /dev/null
+++ b/drivers/vfio/platform/msi/Kconfig
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config VFIO_PLATFORM_BCMPLT_MSI
+   tristate "MSI support for Broadcom platform devices"
+   depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
+   default ARCH_BCM_IPROC
+   help
+ Enables the VFIO platform driver to handle msi for Broadcom devices
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/msi/Makefile 
b/drivers/vfio/platform/msi/Makefile
new file mode 100644
index ..27422d45cecb
--- /dev/null
+++ b/drivers/vfio/platform/msi/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_VFIO_PLATFORM_BCMPLT_MSI) += vfio_platform_bcmplt.o
diff --git a/drivers/vfio/platform/msi/vfio_platform_bcmplt.c 
b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
new file mode 100644
index ..a074b5e92d77
--- /dev/null
+++ b/drivers/vfio/platform/msi/vfio_platform_bcmplt.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020 Broadcom.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../vfio_platform_private.h"
+
+#define RING_SIZE  (64 << 10)
+
+#define RING_MSI_ADDR_LS   0x03c
+#define RING_MSI_ADDR_MS   0x040
+#define RING_MSI_DATA_VALUE0x064
+
+static u32 bcm_num_msi(struct vfio_platform_device *vdev)
+{
+   struct vfio_platform_region *reg = >regions[0];
+
+   return (reg->size / RING_SIZE);
+}
+
+static struct vfio_platform_msi_node vfio_platform_bcmflexrm_msi_node = {
+   .owner = THIS_MODULE,
+   .compat = "brcm,iproc-flexrm-mbox",
+   .of_get_msi = bcm_num_msi,
+};
+
+static int __init vfio_platform_bcmflexrm_msi_module_init(void)
+{
+   __vfio_platform_register_msi(_platform_bcmflexrm_msi_node);
+
+   return 0;
+}
+
+static void __exit vfio_platform_bcmflexrm_msi_module_exit(void)
+{
+   vfio_platform_unregister_msi("brcm,iproc-flexrm-mbox");
+}
+
+module_init(vfio_platform_bcmflexrm_msi_module_init);
+module_exit(vfio_platform_bcmflexrm_msi_module_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Broadcom");
-- 
2.17.1


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.


smime.p7s
Description: S/MIME Cryptographic Signature