All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] treat AHCI controllers under VMD as part of VMD
@ 2023-01-26  2:16 Kevin Friedberg
  2023-02-01  8:54 ` Mariusz Tkaczyk
  2023-02-07  8:44 ` Mariusz Tkaczyk
  0 siblings, 2 replies; 7+ messages in thread
From: Kevin Friedberg @ 2023-01-26  2:16 UTC (permalink / raw)
  To: linux-raid

Detect when a SATA controller has been mapped under Intel Alderlake RST
VMD and list it as part of the domain, instead of independently, so that
it can use the VMD controller's RAID capabilities.

Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>
---
 platform-intel.c | 15 +++++++++------
 super-intel.c    | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/platform-intel.c b/platform-intel.c
index 757f0b1b..859bf743 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -64,10 +64,12 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
 
 	if (strcmp(driver, "isci") == 0)
 		type = SYS_DEV_SAS;
-	else if (strcmp(driver, "ahci") == 0)
+	else if (strcmp(driver, "ahci") == 0) {
+		/* if looking for sata devs, ignore vmd */
+		vmd = find_driver_devices("pci", "vmd");
 		type = SYS_DEV_SATA;
-	else if (strcmp(driver, "nvme") == 0) {
-		/* if looking for nvme devs, first look for vmd */
+	} else if (strcmp(driver, "nvme") == 0) {
+		/* if looking for nvme devs, also look for vmd */
 		vmd = find_driver_devices("pci", "vmd");
 		type = SYS_DEV_NVME;
 	} else if (strcmp(driver, "vmd") == 0)
@@ -104,8 +106,8 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
 		sprintf(path, "/sys/bus/%s/drivers/%s/%s",
 			bus, driver, de->d_name);
 
-		/* if searching for nvme - skip vmd connected one */
-		if (type == SYS_DEV_NVME) {
+		/* if searching for nvme or ahci - skip vmd connected one */
+		if (type == SYS_DEV_NVME || type == SYS_DEV_SATA) {
 			struct sys_dev *dev;
 			char *rp = realpath(path, NULL);
 			for (dev = vmd; dev; dev = dev->next) {
@@ -166,7 +168,8 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
 	}
 	closedir(driver_dir);
 
-	if (vmd) {
+	/* VMD adopts multiple types but should only be listed once */
+	if (vmd && type == SYS_DEV_NVME) {
 		if (list)
 			list->next = vmd;
 		else
diff --git a/super-intel.c b/super-intel.c
index 89fac626..4ef8f0d8 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2680,6 +2680,8 @@ static void print_imsm_capability_export(const struct imsm_orom *orom)
 	printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
 }
 
+#define PCI_CLASS_AHCI_CNTRL "0x010601"
+
 static int detail_platform_imsm(int verbose, int enumerate_only, char *controller_path)
 {
 	/* There are two components to imsm platform support, the ahci SATA
@@ -2752,11 +2754,32 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
 			for (hba = list; hba; hba = hba->next) {
 				if (hba->type == SYS_DEV_VMD) {
 					char buf[PATH_MAX];
+					struct dirent *ent;
+					DIR *dir;
+
 					printf(" I/O Controller : %s (%s)\n",
 						vmd_domain_to_controller(hba, buf), get_sys_dev_type(hba->type));
+					dir = opendir(hba->path);
+					for (ent = readdir(dir); ent; ent = readdir(dir)) {
+						char ent_path[PATH_MAX];
+
+						sprintf(ent_path, "%s/%s", hba->path, ent->d_name);
+						devpath_to_char(ent_path, "class", buf, sizeof(buf), 0);
+						if (strcmp(buf, PCI_CLASS_AHCI_CNTRL) == 0) {
+							host_base = ahci_get_port_count(ent_path, &port_count);
+							if (ahci_enumerate_ports(ent_path, port_count, host_base, verbose)) {
+								if (verbose > 0)
+								pr_err("failed to enumerate ports on VMD SATA controller at %s.\n",
+									hba->pci_id);
+								result |= 2;
+							}
+						}
+					}
+					closedir(dir);
+
 					if (print_nvme_info(hba)) {
 						if (verbose > 0)
-							pr_err("failed to get devices attached to VMD domain.\n");
+							pr_err("failed to get NVMe devices attached to VMD domain.\n");
 						result |= 2;
 					}
 				}
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] treat AHCI controllers under VMD as part of VMD
  2023-01-26  2:16 [PATCH] treat AHCI controllers under VMD as part of VMD Kevin Friedberg
@ 2023-02-01  8:54 ` Mariusz Tkaczyk
  2023-02-05  6:23   ` Kevin Friedberg
  2023-02-07  8:44 ` Mariusz Tkaczyk
  1 sibling, 1 reply; 7+ messages in thread
From: Mariusz Tkaczyk @ 2023-02-01  8:54 UTC (permalink / raw)
  To: Kevin Friedberg; +Cc: linux-raid

Hi Kevin,
Thank you for the patch. Please be aware that the RST is not officially
supported on Linux. You are going to enable RST suppport using VROC Linux
stack. You are using it at your own risk.
Intel will review your patch and will run regression, it could take around
month (sorry, we are getting back our labs now).

Thanks,
Mariusz

On Wed, 25 Jan 2023 21:16:59 -0500
Kevin Friedberg <kev.friedberg@gmail.com> wrote:

> Detect when a SATA controller has been mapped under Intel Alderlake RST
> VMD and list it as part of the domain, instead of independently, so that
> it can use the VMD controller's RAID capabilities.
> 
> Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] treat AHCI controllers under VMD as part of VMD
  2023-02-01  8:54 ` Mariusz Tkaczyk
@ 2023-02-05  6:23   ` Kevin Friedberg
  0 siblings, 0 replies; 7+ messages in thread
From: Kevin Friedberg @ 2023-02-05  6:23 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: linux-raid

Thank you!  Three weeks in it's stable for me (and I do have a backup
just in case). But that's just on my system and having this properly
tested, validated and hopefully merged would be much better.  This is
actually my first time working in C at this level, so any feedback is
appreciated.

Cheers,
Kevin

On Wed, Feb 1, 2023 at 3:54 AM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> Hi Kevin,
> Thank you for the patch. Please be aware that the RST is not officially
> supported on Linux. You are going to enable RST suppport using VROC Linux
> stack. You are using it at your own risk.
> Intel will review your patch and will run regression, it could take around
> month (sorry, we are getting back our labs now).
>
> Thanks,
> Mariusz
>
> On Wed, 25 Jan 2023 21:16:59 -0500
> Kevin Friedberg <kev.friedberg@gmail.com> wrote:
>
> > Detect when a SATA controller has been mapped under Intel Alderlake RST
> > VMD and list it as part of the domain, instead of independently, so that
> > it can use the VMD controller's RAID capabilities.
> >
> > Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] treat AHCI controllers under VMD as part of VMD
  2023-01-26  2:16 [PATCH] treat AHCI controllers under VMD as part of VMD Kevin Friedberg
  2023-02-01  8:54 ` Mariusz Tkaczyk
@ 2023-02-07  8:44 ` Mariusz Tkaczyk
  2023-02-09  0:39   ` Xiao Ni
  2023-02-13  7:02   ` Kevin Friedberg
  1 sibling, 2 replies; 7+ messages in thread
From: Mariusz Tkaczyk @ 2023-02-07  8:44 UTC (permalink / raw)
  To: Kevin Friedberg; +Cc: linux-raid

Hi Kevin,
I found time to take a look into it closer. I think that it is not complete
solution. Please see my comments.

On Wed, 25 Jan 2023 21:16:59 -0500
Kevin Friedberg <kev.friedberg@gmail.com> wrote:

> Detect when a SATA controller has been mapped under Intel Alderlake RST
> VMD and list it as part of the domain, instead of independently, so that
> it can use the VMD controller's RAID capabilities.
> 
> Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>
> ---
>  platform-intel.c | 15 +++++++++------
>  super-intel.c    | 25 ++++++++++++++++++++++++-
>  2 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/platform-intel.c b/platform-intel.c
> index 757f0b1b..859bf743 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -64,10 +64,12 @@ struct sys_dev *find_driver_devices(const char *bus,
> const char *driver) 
>  	if (strcmp(driver, "isci") == 0)
>  		type = SYS_DEV_SAS;
> -	else if (strcmp(driver, "ahci") == 0)
> +	else if (strcmp(driver, "ahci") == 0) {
> +		/* if looking for sata devs, ignore vmd */
> +		vmd = find_driver_devices("pci", "vmd");
>  		type = SYS_DEV_SATA;
> -	else if (strcmp(driver, "nvme") == 0) {
> -		/* if looking for nvme devs, first look for vmd */
> +	} else if (strcmp(driver, "nvme") == 0) {
> +		/* if looking for nvme devs, also look for vmd */
>  		vmd = find_driver_devices("pci", "vmd");
>  		type = SYS_DEV_NVME;
>  	} else if (strcmp(driver, "vmd") == 0)
> @@ -104,8 +106,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> const char *driver) sprintf(path, "/sys/bus/%s/drivers/%s/%s",
>  			bus, driver, de->d_name);
>  
> -		/* if searching for nvme - skip vmd connected one */
> -		if (type == SYS_DEV_NVME) {
> +		/* if searching for nvme or ahci - skip vmd connected one */
> +		if (type == SYS_DEV_NVME || type == SYS_DEV_SATA) {
>  			struct sys_dev *dev;
>  			char *rp = realpath(path, NULL);
>  			for (dev = vmd; dev; dev = dev->next) {
> @@ -166,7 +168,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> const char *driver) }
>  	closedir(driver_dir);
>  
> -	if (vmd) {
> +	/* VMD adopts multiple types but should only be listed once */
> +	if (vmd && type == SYS_DEV_NVME) {
>  		if (list)
>  			list->next = vmd;
>  		else

The SATA behind VMD deserves own type, let say SYS_DEV_SATA_VMD. We cannot use
SYS_DEV_VMD because it will allow to use NVME devices behind VMD in SATA Raid
array. It means that if you have them connected, like:
VMD___ NVME0
    |_ NVME1
    |_ SATA___SATA0
           |__SATA1
You will be able to mix SATA and NVME drives together in RAID. Mdmonitor
could mix them too (if appropriate policy is set). That is not allowed from at
least VROC requirements PoV.

> diff --git a/super-intel.c b/super-intel.c
> index 89fac626..4ef8f0d8 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -2680,6 +2680,8 @@ static void print_imsm_capability_export(const struct
> imsm_orom *orom) printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
>  }
>  
> +#define PCI_CLASS_AHCI_CNTRL "0x010601"
This should be defined in platform-intel.h

> +
>  static int detail_platform_imsm(int verbose, int enumerate_only, char
> *controller_path) {
>  	/* There are two components to imsm platform support, the ahci SATA
> @@ -2752,11 +2754,32 @@ static int detail_platform_imsm(int verbose, int
> enumerate_only, char *controlle for (hba = list; hba; hba = hba->next) {
>  				if (hba->type == SYS_DEV_VMD) {
>  					char buf[PATH_MAX];
> +					struct dirent *ent;
> +					DIR *dir;
> +
>  					printf(" I/O Controller : %s (%s)\n",
>  						vmd_domain_to_controller(hba,
> buf), get_sys_dev_type(hba->type));
> +					dir = opendir(hba->path);
> +					for (ent = readdir(dir); ent; ent =
> readdir(dir)) {
> +						char ent_path[PATH_MAX];
> +
> +						sprintf(ent_path, "%s/%s",
> hba->path, ent->d_name);
> +						devpath_to_char(ent_path,
> "class", buf, sizeof(buf), 0);
> +						if (strcmp(buf,
> PCI_CLASS_AHCI_CNTRL) == 0) {
> +							host_base =
> ahci_get_port_count(ent_path, &port_count);
> +							if
> (ahci_enumerate_ports(ent_path, port_count, host_base, verbose)) {
> +								if (verbose
> > 0)
> +
> pr_err("failed to enumerate ports on VMD SATA controller at %s.\n",
> +
> hba->pci_id);
> +								result |= 2;
> +							}
> +						}
> +					}
> +					closedir(dir);
> +
>  					if (print_nvme_info(hba)) {
>  						if (verbose > 0)
> -							pr_err("failed to
> get devices attached to VMD domain.\n");
> +							pr_err("failed to
> get NVMe devices attached to VMD domain.\n"); result |= 2;
>  					}
>  				}
Similar logic is already there:
https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/tree/super-intel.c#n2780
I would like to reuse it instead of adding new code branch. Could you please
extract similar parts to new function and use it in both places?

Thanks,
Mariusz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] treat AHCI controllers under VMD as part of VMD
  2023-02-07  8:44 ` Mariusz Tkaczyk
@ 2023-02-09  0:39   ` Xiao Ni
  2023-02-10 10:17     ` Mariusz Tkaczyk
  2023-02-13  7:02   ` Kevin Friedberg
  1 sibling, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2023-02-09  0:39 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: Kevin Friedberg, linux-raid

On Tue, Feb 7, 2023 at 4:46 PM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> Hi Kevin,
> I found time to take a look into it closer. I think that it is not complete
> solution. Please see my comments.
>
> On Wed, 25 Jan 2023 21:16:59 -0500
> Kevin Friedberg <kev.friedberg@gmail.com> wrote:
>
> > Detect when a SATA controller has been mapped under Intel Alderlake RST
> > VMD and list it as part of the domain, instead of independently, so that
> > it can use the VMD controller's RAID capabilities.
> >
> > Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>
> > ---
> >  platform-intel.c | 15 +++++++++------
> >  super-intel.c    | 25 ++++++++++++++++++++++++-
> >  2 files changed, 33 insertions(+), 7 deletions(-)
> >
> > diff --git a/platform-intel.c b/platform-intel.c
> > index 757f0b1b..859bf743 100644
> > --- a/platform-intel.c
> > +++ b/platform-intel.c
> > @@ -64,10 +64,12 @@ struct sys_dev *find_driver_devices(const char *bus,
> > const char *driver)
> >       if (strcmp(driver, "isci") == 0)
> >               type = SYS_DEV_SAS;
> > -     else if (strcmp(driver, "ahci") == 0)
> > +     else if (strcmp(driver, "ahci") == 0) {
> > +             /* if looking for sata devs, ignore vmd */
> > +             vmd = find_driver_devices("pci", "vmd");
> >               type = SYS_DEV_SATA;
> > -     else if (strcmp(driver, "nvme") == 0) {
> > -             /* if looking for nvme devs, first look for vmd */
> > +     } else if (strcmp(driver, "nvme") == 0) {
> > +             /* if looking for nvme devs, also look for vmd */
> >               vmd = find_driver_devices("pci", "vmd");
> >               type = SYS_DEV_NVME;
> >       } else if (strcmp(driver, "vmd") == 0)
> > @@ -104,8 +106,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> > const char *driver) sprintf(path, "/sys/bus/%s/drivers/%s/%s",
> >                       bus, driver, de->d_name);
> >
> > -             /* if searching for nvme - skip vmd connected one */
> > -             if (type == SYS_DEV_NVME) {
> > +             /* if searching for nvme or ahci - skip vmd connected one */
> > +             if (type == SYS_DEV_NVME || type == SYS_DEV_SATA) {
> >                       struct sys_dev *dev;
> >                       char *rp = realpath(path, NULL);
> >                       for (dev = vmd; dev; dev = dev->next) {
> > @@ -166,7 +168,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> > const char *driver) }
> >       closedir(driver_dir);
> >
> > -     if (vmd) {
> > +     /* VMD adopts multiple types but should only be listed once */
> > +     if (vmd && type == SYS_DEV_NVME) {
> >               if (list)
> >                       list->next = vmd;
> >               else
>
> The SATA behind VMD deserves own type, let say SYS_DEV_SATA_VMD. We cannot use
> SYS_DEV_VMD because it will allow to use NVME devices behind VMD in SATA Raid
> array. It means that if you have them connected, like:
> VMD___ NVME0
>     |_ NVME1
>     |_ SATA___SATA0
>            |__SATA1
> You will be able to mix SATA and NVME drives together in RAID. Mdmonitor
> could mix them too (if appropriate policy is set). That is not allowed from at
> least VROC requirements PoV.


Hi Mariusz

Through the description of VMD
(https://www.chipict.com/intel_vmd_vroc/), it looks like VMD only
supports pcie nvme devices. Can it also connect sata devices?

And what's PoV?

Best Regards
Xiao


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] treat AHCI controllers under VMD as part of VMD
  2023-02-09  0:39   ` Xiao Ni
@ 2023-02-10 10:17     ` Mariusz Tkaczyk
  0 siblings, 0 replies; 7+ messages in thread
From: Mariusz Tkaczyk @ 2023-02-10 10:17 UTC (permalink / raw)
  To: Xiao Ni; +Cc: Kevin Friedberg, linux-raid

On Thu, 9 Feb 2023 08:39:21 +0800
Xiao Ni <xni@redhat.com> wrote:

> On Tue, Feb 7, 2023 at 4:46 PM Mariusz Tkaczyk
> <mariusz.tkaczyk@linux.intel.com> wrote:
> >
> > Hi Kevin,
> > I found time to take a look into it closer. I think that it is not complete
> > solution. Please see my comments.
> >
> > On Wed, 25 Jan 2023 21:16:59 -0500
> > Kevin Friedberg <kev.friedberg@gmail.com> wrote:
> >  
> > > Detect when a SATA controller has been mapped under Intel Alderlake RST
> > > VMD and list it as part of the domain, instead of independently, so that
> > > it can use the VMD controller's RAID capabilities.
> > >
> > > Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>
> > > ---
> > >  platform-intel.c | 15 +++++++++------
> > >  super-intel.c    | 25 ++++++++++++++++++++++++-
> > >  2 files changed, 33 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/platform-intel.c b/platform-intel.c
> > > index 757f0b1b..859bf743 100644
> > > --- a/platform-intel.c
> > > +++ b/platform-intel.c
> > > @@ -64,10 +64,12 @@ struct sys_dev *find_driver_devices(const char *bus,
> > > const char *driver)
> > >       if (strcmp(driver, "isci") == 0)
> > >               type = SYS_DEV_SAS;
> > > -     else if (strcmp(driver, "ahci") == 0)
> > > +     else if (strcmp(driver, "ahci") == 0) {
> > > +             /* if looking for sata devs, ignore vmd */
> > > +             vmd = find_driver_devices("pci", "vmd");
> > >               type = SYS_DEV_SATA;
> > > -     else if (strcmp(driver, "nvme") == 0) {
> > > -             /* if looking for nvme devs, first look for vmd */
> > > +     } else if (strcmp(driver, "nvme") == 0) {
> > > +             /* if looking for nvme devs, also look for vmd */
> > >               vmd = find_driver_devices("pci", "vmd");
> > >               type = SYS_DEV_NVME;
> > >       } else if (strcmp(driver, "vmd") == 0)
> > > @@ -104,8 +106,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> > > const char *driver) sprintf(path, "/sys/bus/%s/drivers/%s/%s",
> > >                       bus, driver, de->d_name);
> > >
> > > -             /* if searching for nvme - skip vmd connected one */
> > > -             if (type == SYS_DEV_NVME) {
> > > +             /* if searching for nvme or ahci - skip vmd connected one */
> > > +             if (type == SYS_DEV_NVME || type == SYS_DEV_SATA) {
> > >                       struct sys_dev *dev;
> > >                       char *rp = realpath(path, NULL);
> > >                       for (dev = vmd; dev; dev = dev->next) {
> > > @@ -166,7 +168,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> > > const char *driver) }
> > >       closedir(driver_dir);
> > >
> > > -     if (vmd) {
> > > +     /* VMD adopts multiple types but should only be listed once */
> > > +     if (vmd && type == SYS_DEV_NVME) {
> > >               if (list)
> > >                       list->next = vmd;
> > >               else  
> >
> > The SATA behind VMD deserves own type, let say SYS_DEV_SATA_VMD. We cannot
> > use SYS_DEV_VMD because it will allow to use NVME devices behind VMD in
> > SATA Raid array. It means that if you have them connected, like:
> > VMD___ NVME0
> >     |_ NVME1
> >     |_ SATA___SATA0
> >            |__SATA1
> > You will be able to mix SATA and NVME drives together in RAID. Mdmonitor
> > could mix them too (if appropriate policy is set). That is not allowed from
> > at least VROC requirements PoV.  
> 
> 
> Hi Mariusz
> 
> Through the description of VMD
> (https://www.chipict.com/intel_vmd_vroc/), it looks like VMD only
> supports pcie nvme devices. Can it also connect sata devices?
> 
> And what's PoV?
> 
Hi Xiao,
Not directly, there must be SATA controller behind VMD.
In VROC on server platforms we don't use that. So far I know, the reason is to
have only one driver to handle all RST devices but it could be only part of
story.

Thanks,
Mariusz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] treat AHCI controllers under VMD as part of VMD
  2023-02-07  8:44 ` Mariusz Tkaczyk
  2023-02-09  0:39   ` Xiao Ni
@ 2023-02-13  7:02   ` Kevin Friedberg
  1 sibling, 0 replies; 7+ messages in thread
From: Kevin Friedberg @ 2023-02-13  7:02 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: linux-raid

Hi Mariusz,

Thank you for the feedback.  I'm working up a replacement patch based
on your comments and should have it ready in the next couple days.  It
works in a different way and has a different commit message, so go
ahead and reject this patch if there's a process for that.

On Tue, Feb 7, 2023 at 3:44 AM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> Hi Kevin,
> I found time to take a look into it closer. I think that it is not complete
> solution. Please see my comments.
>
> On Wed, 25 Jan 2023 21:16:59 -0500
> Kevin Friedberg <kev.friedberg@gmail.com> wrote:
>
> > Detect when a SATA controller has been mapped under Intel Alderlake RST
> > VMD and list it as part of the domain, instead of independently, so that
> > it can use the VMD controller's RAID capabilities.
> >
> > Signed-off-by: Kevin Friedberg <kev.friedberg@gmail.com>
> > ---
> >  platform-intel.c | 15 +++++++++------
> >  super-intel.c    | 25 ++++++++++++++++++++++++-
> >  2 files changed, 33 insertions(+), 7 deletions(-)
> >
> > diff --git a/platform-intel.c b/platform-intel.c
> > index 757f0b1b..859bf743 100644
> > --- a/platform-intel.c
> > +++ b/platform-intel.c
> > @@ -64,10 +64,12 @@ struct sys_dev *find_driver_devices(const char *bus,
> > const char *driver)
> >       if (strcmp(driver, "isci") == 0)
> >               type = SYS_DEV_SAS;
> > -     else if (strcmp(driver, "ahci") == 0)
> > +     else if (strcmp(driver, "ahci") == 0) {
> > +             /* if looking for sata devs, ignore vmd */
> > +             vmd = find_driver_devices("pci", "vmd");
> >               type = SYS_DEV_SATA;
> > -     else if (strcmp(driver, "nvme") == 0) {
> > -             /* if looking for nvme devs, first look for vmd */
> > +     } else if (strcmp(driver, "nvme") == 0) {
> > +             /* if looking for nvme devs, also look for vmd */
> >               vmd = find_driver_devices("pci", "vmd");
> >               type = SYS_DEV_NVME;
> >       } else if (strcmp(driver, "vmd") == 0)
> > @@ -104,8 +106,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> > const char *driver) sprintf(path, "/sys/bus/%s/drivers/%s/%s",
> >                       bus, driver, de->d_name);
> >
> > -             /* if searching for nvme - skip vmd connected one */
> > -             if (type == SYS_DEV_NVME) {
> > +             /* if searching for nvme or ahci - skip vmd connected one */
> > +             if (type == SYS_DEV_NVME || type == SYS_DEV_SATA) {
> >                       struct sys_dev *dev;
> >                       char *rp = realpath(path, NULL);
> >                       for (dev = vmd; dev; dev = dev->next) {
> > @@ -166,7 +168,8 @@ struct sys_dev *find_driver_devices(const char *bus,
> > const char *driver) }
> >       closedir(driver_dir);
> >
> > -     if (vmd) {
> > +     /* VMD adopts multiple types but should only be listed once */
> > +     if (vmd && type == SYS_DEV_NVME) {
> >               if (list)
> >                       list->next = vmd;
> >               else
>
> The SATA behind VMD deserves own type, let say SYS_DEV_SATA_VMD. We cannot use
> SYS_DEV_VMD because it will allow to use NVME devices behind VMD in SATA Raid
> array. It means that if you have them connected, like:
> VMD___ NVME0
>     |_ NVME1
>     |_ SATA___SATA0
>            |__SATA1
> You will be able to mix SATA and NVME drives together in RAID. Mdmonitor
> could mix them too (if appropriate policy is set). That is not allowed from at
> least VROC requirements PoV.
>
> > diff --git a/super-intel.c b/super-intel.c
> > index 89fac626..4ef8f0d8 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -2680,6 +2680,8 @@ static void print_imsm_capability_export(const struct
> > imsm_orom *orom) printf("IMSM_MAX_VOLUMES_PER_CONTROLLER=%d\n",orom->vphba);
> >  }
> >
> > +#define PCI_CLASS_AHCI_CNTRL "0x010601"
> This should be defined in platform-intel.h
>
> > +
> >  static int detail_platform_imsm(int verbose, int enumerate_only, char
> > *controller_path) {
> >       /* There are two components to imsm platform support, the ahci SATA
> > @@ -2752,11 +2754,32 @@ static int detail_platform_imsm(int verbose, int
> > enumerate_only, char *controlle for (hba = list; hba; hba = hba->next) {
> >                               if (hba->type == SYS_DEV_VMD) {
> >                                       char buf[PATH_MAX];
> > +                                     struct dirent *ent;
> > +                                     DIR *dir;
> > +
> >                                       printf(" I/O Controller : %s (%s)\n",
> >                                               vmd_domain_to_controller(hba,
> > buf), get_sys_dev_type(hba->type));
> > +                                     dir = opendir(hba->path);
> > +                                     for (ent = readdir(dir); ent; ent =
> > readdir(dir)) {
> > +                                             char ent_path[PATH_MAX];
> > +
> > +                                             sprintf(ent_path, "%s/%s",
> > hba->path, ent->d_name);
> > +                                             devpath_to_char(ent_path,
> > "class", buf, sizeof(buf), 0);
> > +                                             if (strcmp(buf,
> > PCI_CLASS_AHCI_CNTRL) == 0) {
> > +                                                     host_base =
> > ahci_get_port_count(ent_path, &port_count);
> > +                                                     if
> > (ahci_enumerate_ports(ent_path, port_count, host_base, verbose)) {
> > +                                                             if (verbose
> > > 0)
> > +
> > pr_err("failed to enumerate ports on VMD SATA controller at %s.\n",
> > +
> > hba->pci_id);
> > +                                                             result |= 2;
> > +                                                     }
> > +                                             }
> > +                                     }
> > +                                     closedir(dir);
> > +
> >                                       if (print_nvme_info(hba)) {
> >                                               if (verbose > 0)
> > -                                                     pr_err("failed to
> > get devices attached to VMD domain.\n");
> > +                                                     pr_err("failed to
> > get NVMe devices attached to VMD domain.\n"); result |= 2;
> >                                       }
> >                               }
> Similar logic is already there:
> https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/tree/super-intel.c#n2780
> I would like to reuse it instead of adding new code branch. Could you please
> extract similar parts to new function and use it in both places?
>
> Thanks,
> Mariusz

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-02-13  7:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-26  2:16 [PATCH] treat AHCI controllers under VMD as part of VMD Kevin Friedberg
2023-02-01  8:54 ` Mariusz Tkaczyk
2023-02-05  6:23   ` Kevin Friedberg
2023-02-07  8:44 ` Mariusz Tkaczyk
2023-02-09  0:39   ` Xiao Ni
2023-02-10 10:17     ` Mariusz Tkaczyk
2023-02-13  7:02   ` Kevin Friedberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.