2006-02-20 18:02:14

by Dax Kelson

[permalink] [raw]
Subject: Areca RAID driver remaining items?

This appears to be the most current version of the driver:

ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.16-rc4/2.6.16-rc4-mm1/broken-out/areca-raid-linux-scsi-driver.patch

Is this the current TODO list?

=================
Issues not yet patched:

13. uintNN_t int types: use kernel types except for userspace
interfaces
14. use kernel-doc
18. Put arcmsr.txt in Documentation/scsi/, not in scsi/arcmsr/.
19. Maybe use sysfs (/sys) instead of /proc.
20. check stack usage, init/exit sections;
=================

At one point this comment was made:

"There's lots of architectural problems. It's doing it's own queueing,
it's stuffing kernel structures into memory on the hardware and so on.
Basically someone knowledgeable about the hardware needs to start from
scratch on it."

What are the show stoppers that prevents a merge into the Linus tree?

Dax Kelson


2006-02-20 18:11:26

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?


> "There's lots of architectural problems. It's doing it's own queueing,
> it's stuffing kernel structures into memory on the hardware and so on.
> Basically someone knowledgeable about the hardware needs to start from
> scratch on it."
>
> What are the show stoppers that prevents a merge into the Linus tree?

own queueing for sure is a show stopper;
stuffing kernel structures into the hw is also very much evil


2006-02-20 18:20:50

by Christoph Hellwig

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Mon, Feb 20, 2006 at 11:02:32AM -0700, Dax Kelson wrote:
> This appears to be the most current version of the driver:
>
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.16-rc4/2.6.16-rc4-mm1/broken-out/areca-raid-linux-scsi-driver.patch
>
> Is this the current TODO list?
>
> =================
> Issues not yet patched:
>
> 13. uintNN_t int types: use kernel types except for userspace
> interfaces
> 14. use kernel-doc
> 18. Put arcmsr.txt in Documentation/scsi/, not in scsi/arcmsr/.
> 19. Maybe use sysfs (/sys) instead of /proc.
> 20. check stack usage, init/exit sections;


- remove internal queueing
- fix hardware datastructures
- remove odd ioctls
- remove useless forward prototypes
- give types like ACB useful names
- give variable useful names, especially follow kernel conventions,
e.g. a struct pci_dev is usually named pdev
- kill ->proc_info method
- use normal comment style even for comments not fitting into the
kernel-doc item above. kill useless separator comments without
text
- convert arcmsr_show_firmware_info to useful one value per
file attributes. best follow the schemes used in aacraid or
lpfc
- convert arcmsr_show_driver_state to useful one value per
file attributes.
- remove never called release method in the host template
- audit whether setting unchecked_isa_dma to false really makes
sense (I strongly doubt it)
- remove shutdown notifier, add pci_driver ->shutdown method instead
- remove CameCase PCI Ids. The vendor Id should go into pci_ids.h,
the device ids either removed or spelled the normal linux way
- arcmsr_do_interrupt should stop walking the global host list
and use the private data passed to request_irq
- the global host list should go away completely
- arcmsr_bios_param looks like duplicating the generic CAM version?
- locking needs to be redone. If the driver really needs more than
one per-host lock we'll want a very good explanation
- arcmsr_device_probe needs to be rewritten to do goto-based
error unwinding.
- msi should be a module options if at all, but defintitly not
a config options
- arcmsr_scsi_host_template_init should go away. the host template
must be initialized statically with no run-time writes to it
- the hardware documentation should be split out of arcmsr.h
into a separate file (btw, thanks a lot to areca to provide such
detailed hardware informations, it's just the wrong format..)
- remove the SCSISTAT_* defines, and use the generic ones from
<scsi/scsi.h> instead. Dito for various other SAM defines.
- the driver has just two files and should go directly into
drivers/scsi instead of a subdirectory

2006-02-22 06:29:18

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Hi Christoph Hellwig,

Thanks for your comment with "arcmsr".
I will follow your comment to redo this driver.
But I am confuse with your mention about some items.
Hope you can tell me more detail and let me realy know your comment.

1- remove internal queueing:

Does the "internal queueing" is mention with arcmsr of ccb_free_list ?

2- fix hardware datastructures:

Does the "fix hardware datastructures" is to fix struct ARCMSR_CDB?
Is it illeagal in linux?

3- remove odd ioctls:

How about remove odd ioctl?

Best Regards
Erich Chen

----- Original Message -----
From: "Christoph Hellwig" <[email protected]>
To: "Dax Kelson" <[email protected]>
Cc: <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>
Sent: Tuesday, February 21, 2006 2:20 AM
Subject: Re: Areca RAID driver remaining items?


> On Mon, Feb 20, 2006 at 11:02:32AM -0700, Dax Kelson wrote:
>> This appears to be the most current version of the driver:
>>
>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.16-rc4/2.6.16-rc4-mm1/broken-out/areca-raid-linux-scsi-driver.patch
>>
>> Is this the current TODO list?
>>
>> =================
>> Issues not yet patched:
>>
>> 13. uintNN_t int types: use kernel types except for userspace
>> interfaces
>> 14. use kernel-doc
>> 18. Put arcmsr.txt in Documentation/scsi/, not in scsi/arcmsr/.
>> 19. Maybe use sysfs (/sys) instead of /proc.
>> 20. check stack usage, init/exit sections;
>
>
> - remove internal queueing
> - fix hardware datastructures
> - remove odd ioctls
> - remove useless forward prototypes
> - give types like ACB useful names
> - give variable useful names, especially follow kernel conventions,
> e.g. a struct pci_dev is usually named pdev
> - kill ->proc_info method
> - use normal comment style even for comments not fitting into the
> kernel-doc item above. kill useless separator comments without
> text
> - convert arcmsr_show_firmware_info to useful one value per
> file attributes. best follow the schemes used in aacraid or
> lpfc
> - convert arcmsr_show_driver_state to useful one value per
> file attributes.
> - remove never called release method in the host template
> - audit whether setting unchecked_isa_dma to false really makes
> sense (I strongly doubt it)
> - remove shutdown notifier, add pci_driver ->shutdown method instead
> - remove CameCase PCI Ids. The vendor Id should go into pci_ids.h,
> the device ids either removed or spelled the normal linux way
> - arcmsr_do_interrupt should stop walking the global host list
> and use the private data passed to request_irq
> - the global host list should go away completely
> - arcmsr_bios_param looks like duplicating the generic CAM version?
> - locking needs to be redone. If the driver really needs more than
> one per-host lock we'll want a very good explanation
> - arcmsr_device_probe needs to be rewritten to do goto-based
> error unwinding.
> - msi should be a module options if at all, but defintitly not
> a config options
> - arcmsr_scsi_host_template_init should go away. the host template
> must be initialized statically with no run-time writes to it
> - the hardware documentation should be split out of arcmsr.h
> into a separate file (btw, thanks a lot to areca to provide such
> detailed hardware informations, it's just the wrong format..)
> - remove the SCSISTAT_* defines, and use the generic ones from
> <scsi/scsi.h> instead. Dito for various other SAM defines.
> - the driver has just two files and should go directly into
> drivers/scsi instead of a subdirectory

2006-02-22 14:57:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Wed, Feb 22, 2006 at 02:27:32PM +0800, erich wrote:
> Hi Christoph Hellwig,
>
> Thanks for your comment with "arcmsr".
> I will follow your comment to redo this driver.
> But I am confuse with your mention about some items.
> Hope you can tell me more detail and let me realy know your comment.
>
> 1- remove internal queueing:
>
> Does the "internal queueing" is mention with arcmsr of ccb_free_list ?

Currently the drivers queuecommand routine works the following:

1) perform some checks
2) try to post outstanding ccbs
3) grab new ccb from freelist and set it up
4) try to post new ccb, else enqueue it

there is not poin in having such a pending queue in the driver because
the midlayer does that work for you. If ->queuecommand can't immediately
post a ccb you should return

SCSI_MLQUEUE_HOST_BUSY if there is a resource shortage at the hba level
SCSI_MLQUEUE_DEVICE_BUSY if there is a resource shortage at the device level

and the scsi midlayer will try to send the command again once a command
has been completed on the hba/device.

> 2- fix hardware datastructures:
>
> Does the "fix hardware datastructures" is to fix struct ARCMSR_CDB?
> Is it illeagal in linux?

struct CCB is a structure that is passed to the hardware but contains
pointers which have different sized on different architectures. This
is generally very dangerous. If this is just a cookie that the hardware
doesn't interpret at all it needs more documentation. Also the way
you try to convert from bus to virtual addresses with pACB->vir2phy_offset
can't work on many linux platforms because the virtual to bus address
mapping isn't contingous. you need a separate dma mapping for each ccb,
a good way to archive that is the dma_pool_ * API.


> 3- remove odd ioctls:
>
> How about remove odd ioctl?

generally we don't want to add new ioctls. For scsi/raid drivers there's
been an exception where we allow a pass-through to the firmware which
the managment applications need. the driver has various ioctls that
don't seem to fall into that category.

2006-02-23 06:27:36

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Christoph Hellwig,

I have figure out your comments about "remove internal queueing" and "remove
odd ioctl".
But about "hardware datastructures", areca's firmware spec is need to get a
trunk of contingous memory space under 4G.
In 64bit platform arcmsr need to make sure all ccbs have same of
ccb_phyaddr_hi32 physical address.
If arcmsr use dma_pool_alloc do a separate dma mapping.
Is there any method to avoid ccbs pool cross 4G segment?

- msi should be a module options if at all, but defintitly not
a config options

In some mainboard if I always enable msi function, it will cause system hang
up.
If it is not a config option, do you have any idea to avoid this issue?

Best Regards
Erich Chen
----- Original Message -----
From: "Christoph Hellwig" <[email protected]>
To: "erich" <[email protected]>
Cc: <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>
Sent: Wednesday, February 22, 2006 10:57 PM
Subject: Re: Areca RAID driver remaining items?


> On Wed, Feb 22, 2006 at 02:27:32PM +0800, erich wrote:
>> Hi Christoph Hellwig,
>>
>> Thanks for your comment with "arcmsr".
>> I will follow your comment to redo this driver.
>> But I am confuse with your mention about some items.
>> Hope you can tell me more detail and let me realy know your comment.
>>
>> 1- remove internal queueing:
>>
>> Does the "internal queueing" is mention with arcmsr of ccb_free_list
>> ?
>
> Currently the drivers queuecommand routine works the following:
>
> 1) perform some checks
> 2) try to post outstanding ccbs
> 3) grab new ccb from freelist and set it up
> 4) try to post new ccb, else enqueue it
>
> there is not poin in having such a pending queue in the driver because
> the midlayer does that work for you. If ->queuecommand can't immediately
> post a ccb you should return
>
> SCSI_MLQUEUE_HOST_BUSY if there is a resource shortage at the hba level
> SCSI_MLQUEUE_DEVICE_BUSY if there is a resource shortage at the device
> level
>
> and the scsi midlayer will try to send the command again once a command
> has been completed on the hba/device.
>
>> 2- fix hardware datastructures:
>>
>> Does the "fix hardware datastructures" is to fix struct ARCMSR_CDB?
>> Is it illeagal in linux?
>
> struct CCB is a structure that is passed to the hardware but contains
> pointers which have different sized on different architectures. This
> is generally very dangerous. If this is just a cookie that the hardware
> doesn't interpret at all it needs more documentation. Also the way
> you try to convert from bus to virtual addresses with pACB->vir2phy_offset
> can't work on many linux platforms because the virtual to bus address
> mapping isn't contingous. you need a separate dma mapping for each ccb,
> a good way to archive that is the dma_pool_ * API.
>
>
>> 3- remove odd ioctls:
>>
>> How about remove odd ioctl?
>
> generally we don't want to add new ioctls. For scsi/raid drivers there's
> been an exception where we allow a pass-through to the firmware which
> the managment applications need. the driver has various ioctls that
> don't seem to fall into that category.

2006-02-23 08:26:04

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Thu, 2006-02-23 at 14:27 +0800, erich wrote:
> Dear Christoph Hellwig,
>
> I have figure out your comments about "remove internal queueing" and "remove
> odd ioctl".
> But about "hardware datastructures", areca's firmware spec is need to get a
> trunk of contingous memory space under 4G.
> In 64bit platform arcmsr need to make sure all ccbs have same of
> ccb_phyaddr_hi32 physical address.
> If arcmsr use dma_pool_alloc do a separate dma mapping.
> Is there any method to avoid ccbs pool cross 4G segment?

the pci mapping layer prevents that already entirely; there is a LOT of
hardware that cannot deal with segments crossing 4G boundaries, so much
in fact that it's now generically disabled.


> In some mainboard if I always enable msi function, it will cause system hang
> up.
> If it is not a config option, do you have any idea to avoid this issue?

how about a module option (module_param)?


2006-02-23 09:50:40

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Arjan van de Ven,

The following contex is coming from comment of Christoph Hellwig.

- msi should be a module options if at all, but defintitly not
a config options

#ifdef CONFIG_SCSI_ARCMSR_MSI
if (!pci_enable_msi(pci_device))
pACB->acb_flags |= ACB_F_HAVE_MSI;
#endif

I make an option config for prevent some mainboards hang up if arcmsr enable
msi function.
Areca RAID controller is bridged hardware.
There were a lots of mainboards had wrong IRQ routing table issue with it.
If somebody meet this issue and people can enable msi function to fix its
hardware bug.
But unfortunately I found some mainboards will hang up if I always enable
this function in my lab.
To avoid this issue, I do an option for this case.

But Christoph Hellwig give me comment with it.

Best Regards
Erich Chen

----- Original Message -----
From: "Arjan van de Ven" <[email protected]>
To: "erich" <[email protected]>
Cc: "Christoph Hellwig" <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>
Sent: Thursday, February 23, 2006 4:25 PM
Subject: Re: Areca RAID driver remaining items?


> On Thu, 2006-02-23 at 14:27 +0800, erich wrote:
>> Dear Christoph Hellwig,
>>
>> I have figure out your comments about "remove internal queueing" and
>> "remove
>> odd ioctl".
>> But about "hardware datastructures", areca's firmware spec is need to get
>> a
>> trunk of contingous memory space under 4G.
>> In 64bit platform arcmsr need to make sure all ccbs have same of
>> ccb_phyaddr_hi32 physical address.
>> If arcmsr use dma_pool_alloc do a separate dma mapping.
>> Is there any method to avoid ccbs pool cross 4G segment?
>
> the pci mapping layer prevents that already entirely; there is a LOT of
> hardware that cannot deal with segments crossing 4G boundaries, so much
> in fact that it's now generically disabled.
>
>
>> In some mainboard if I always enable msi function, it will cause system
>> hang
>> up.
>> If it is not a config option, do you have any idea to avoid this issue?
>
> how about a module option (module_param)?
>
>

2006-02-23 09:56:16

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Thu, 2006-02-23 at 17:50 +0800, erich wrote:
> Dear Arjan van de Ven,
>
> The following contex is coming from comment of Christoph Hellwig.
>
> - msi should be a module options if at all, but defintitly not
> a config options
>
> #ifdef CONFIG_SCSI_ARCMSR_MSI
> if (!pci_enable_msi(pci_device))
> pACB->acb_flags |= ACB_F_HAVE_MSI;
> #endif
>
> I make an option config for prevent some mainboards hang up if arcmsr enable
> msi function.
> Areca RAID controller is bridged hardware.
> There were a lots of mainboards had wrong IRQ routing table issue with it.
> If somebody meet this issue and people can enable msi function to fix its
> hardware bug.
> But unfortunately I found some mainboards will hang up if I always enable
> this function in my lab.
> To avoid this issue, I do an option for this case.


yes the reason for making this optional is clear, and Christoph also
understands that.

However the idea that Christoph is proposing is to not make it a compile
time option, but a runtime option. Compile-time is not very flexible,
especially not for linux distributions. Making it a module option means
it becomes runtime behavior, and the user can load the module like

modprobe aerca msi=0

and msi gets turned off. No need to recompile anything! That has many
advantages over a more inflexible (from the user view) compiletime-only
option.


2006-02-23 11:51:47

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Arjan van de Ven,

Thanks for your answer.
I will remove CONFIG_SCSI_ARCMSR_MSI in next patch.

If Linux can not assurent the contingous memory space allocating of
"dma_alloc_coherent" .
When arcmsr get a physical ccb address from areca's firmware.
Does linux has any functions for converting of "bus to virtual" ?

Best Regards
Erich Chen

----- Original Message -----
From: "Arjan van de Ven" <[email protected]>
To: "erich" <[email protected]>
Cc: ""Christoph Hellwig"" <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>
Sent: Thursday, February 23, 2006 5:56 PM
Subject: Re: Areca RAID driver remaining items?


> On Thu, 2006-02-23 at 17:50 +0800, erich wrote:
>> Dear Arjan van de Ven,
>>
>> The following contex is coming from comment of Christoph Hellwig.
>>
>> - msi should be a module options if at all, but defintitly not
>> a config options
>>
>> #ifdef CONFIG_SCSI_ARCMSR_MSI
>> if (!pci_enable_msi(pci_device))
>> pACB->acb_flags |= ACB_F_HAVE_MSI;
>> #endif
>>
>> I make an option config for prevent some mainboards hang up if arcmsr
>> enable
>> msi function.
>> Areca RAID controller is bridged hardware.
>> There were a lots of mainboards had wrong IRQ routing table issue with
>> it.
>> If somebody meet this issue and people can enable msi function to fix its
>> hardware bug.
>> But unfortunately I found some mainboards will hang up if I always enable
>> this function in my lab.
>> To avoid this issue, I do an option for this case.
>
>
> yes the reason for making this optional is clear, and Christoph also
> understands that.
>
> However the idea that Christoph is proposing is to not make it a compile
> time option, but a runtime option. Compile-time is not very flexible,
> especially not for linux distributions. Making it a module option means
> it becomes runtime behavior, and the user can load the module like
>
> modprobe aerca msi=0
>
> and msi gets turned off. No need to recompile anything! That has many
> advantages over a more inflexible (from the user view) compiletime-only
> option.
>
>

2006-02-23 11:55:57

by Alan

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Iau, 2006-02-23 at 17:50 +0800, erich wrote:
> But unfortunately I found some mainboards will hang up if I always enable
> this function in my lab.
> To avoid this issue, I do an option for this case.
>
> But Christoph Hellwig give me comment with it.


Another thing you can also do for many of these cases is to use either
the PCI or DMI interfaces to identify the problem board and
automatically set the option as well.

There are two ways to do this. One is

struct pci_dev *bridge_dev = pci_get_slot(pdev->bus, PCI_DEVFN(0,0));
if(bridge_dev) {
if(bridge_dev->subsystem_vendor == 0xXXXX &&
bridge_dev->subsystem_device == 0xXXXX)
/* Match by svid/sdid for problem boards */

The other is like this

#include <linux/dmi.h>

struct dmi_system_id problem_dmi_table[] = {
{
.ident = "Broken Board Name 1",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "EvilCorp");
DMI_MATCH(DMI_PRODUCTNAME, "Wombat 1000");
}
}
{
ditto per board
},
{ } /* End of list mark
};


And then

if (dmi_system_check(problem_dmi_table))
disable_msi..


The DMI code matches on the DMI strings in the ROM BIOS (the ones dumped
by 'dmidecode')


An example driver using this interface is drivers/char/sonypi.c which
uses it to make sure it *is* only run on a sony laptop.

Alan

2006-02-23 12:07:38

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Thu, 2006-02-23 at 19:51 +0800, erich wrote:
> If Linux can not assurent the contingous memory space allocating of
> "dma_alloc_coherent" .

coherent memory is guaranteed to be in the "lower" 32 bit of memory!
So that is good news, I think you are just fine.

[Exception is that you can say that you are ok with a bigger mask for
this type of memory, but just don't do that if you're not]


> When arcmsr get a physical ccb address from areca's firmware.
> Does linux has any functions for converting of "bus to virtual" ?

not without using pools. You would have to search the list of memory you
gave it to find that out.

(USB has a similar problem, afaik they solved it with pools)


2006-02-24 02:08:23

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Arjan van de Ven,

I would keep dma_alloc_coherent usage.

> [Exception is that you can say that you are ok with a bigger mask for
> this type of memory, but just don't do that if you're not]

Should I remove "pci_set_dma_mask(pci_device, DMA_64BIT_MASK)" for this
case?

Best Regards
Erich Chen

----- Original Message -----
From: "Arjan van de Ven" <[email protected]>
To: "erich" <[email protected]>
Cc: """Christoph Hellwig""" <[email protected]>;
<[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>;
<[email protected]>
Sent: Thursday, February 23, 2006 8:07 PM
Subject: Re: Areca RAID driver remaining items?


> On Thu, 2006-02-23 at 19:51 +0800, erich wrote:
>> If Linux can not assurent the contingous memory space allocating of
>> "dma_alloc_coherent" .
>
> coherent memory is guaranteed to be in the "lower" 32 bit of memory!
> So that is good news, I think you are just fine.
>
> [Exception is that you can say that you are ok with a bigger mask for
> this type of memory, but just don't do that if you're not]
>
>
>> When arcmsr get a physical ccb address from areca's firmware.
>> Does linux has any functions for converting of "bus to virtual" ?
>
> not without using pools. You would have to search the list of memory you
> gave it to find that out.
>
> (USB has a similar problem, afaik they solved it with pools)
>
>

2006-02-24 02:36:37

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Alan Cox,

This is good idea to make black list to prevent system hang up with MSI
function.
But arcmsr need to come up against none specific mainboards.
The case at my lab have same chipset but different maker.

Best Regards
Erich Chen

----- Original Message -----
From: "Alan Cox" <[email protected]>
To: "erich" <[email protected]>
Cc: "Arjan van de Ven" <[email protected]>; ""Christoph Hellwig""
<[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>;
<[email protected]>
Sent: Thursday, February 23, 2006 7:59 PM
Subject: Re: Areca RAID driver remaining items?


> On Iau, 2006-02-23 at 17:50 +0800, erich wrote:
>> But unfortunately I found some mainboards will hang up if I always enable
>> this function in my lab.
>> To avoid this issue, I do an option for this case.
>>
>> But Christoph Hellwig give me comment with it.
>
>
> Another thing you can also do for many of these cases is to use either
> the PCI or DMI interfaces to identify the problem board and
> automatically set the option as well.
>
> There are two ways to do this. One is
>
> struct pci_dev *bridge_dev = pci_get_slot(pdev->bus, PCI_DEVFN(0,0));
> if(bridge_dev) {
> if(bridge_dev->subsystem_vendor == 0xXXXX &&
> bridge_dev->subsystem_device == 0xXXXX)
> /* Match by svid/sdid for problem boards */
>
> The other is like this
>
> #include <linux/dmi.h>
>
> struct dmi_system_id problem_dmi_table[] = {
> {
> .ident = "Broken Board Name 1",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "EvilCorp");
> DMI_MATCH(DMI_PRODUCTNAME, "Wombat 1000");
> }
> }
> {
> ditto per board
> },
> { } /* End of list mark
> };
>
>
> And then
>
> if (dmi_system_check(problem_dmi_table))
> disable_msi..
>
>
> The DMI code matches on the DMI strings in the ROM BIOS (the ones dumped
> by 'dmidecode')
>
>
> An example driver using this interface is drivers/char/sonypi.c which
> uses it to make sure it *is* only run on a sony laptop.
>
> Alan
>

2006-02-24 03:18:18

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Arjan van de Ven,
I had misconstruction with
>> [Exception is that you can say that you are ok with a bigger mask for
>> this type of memory, but just don't do that if you're not]

it should be "pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK)."

Best Regards
Erich Chen

----- Original Message -----
From: "erich" <[email protected]>
To: "Arjan van de Ven" <[email protected]>
Cc: """"Christoph Hellwig"""" <[email protected]>;
<[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>;
<[email protected]>
Sent: Friday, February 24, 2006 10:08 AM
Subject: Re: Areca RAID driver remaining items?


> Dear Arjan van de Ven,
>
> I would keep dma_alloc_coherent usage.
>
>> [Exception is that you can say that you are ok with a bigger mask for
>> this type of memory, but just don't do that if you're not]
>
> Should I remove "pci_set_dma_mask(pci_device, DMA_64BIT_MASK)" for this
> case?
>
> Best Regards
> Erich Chen
>
> ----- Original Message -----
> From: "Arjan van de Ven" <[email protected]>
> To: "erich" <[email protected]>
> Cc: """Christoph Hellwig""" <[email protected]>;
> <[email protected]>; <[email protected]>;
> <[email protected]>; <[email protected]>; <[email protected]>;
> <[email protected]>
> Sent: Thursday, February 23, 2006 8:07 PM
> Subject: Re: Areca RAID driver remaining items?
>
>
>> On Thu, 2006-02-23 at 19:51 +0800, erich wrote:
>>> If Linux can not assurent the contingous memory space allocating of
>>> "dma_alloc_coherent" .
>>
>> coherent memory is guaranteed to be in the "lower" 32 bit of memory!
>> So that is good news, I think you are just fine.
>>
>> [Exception is that you can say that you are ok with a bigger mask for
>> this type of memory, but just don't do that if you're not]
>>
>>
>>> When arcmsr get a physical ccb address from areca's firmware.
>>> Does linux has any functions for converting of "bus to virtual" ?
>>
>> not without using pools. You would have to search the list of memory you
>> gave it to find that out.
>>
>> (USB has a similar problem, afaik they solved it with pools)
>>
>>
>

2006-02-24 08:50:48

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Fri, 2006-02-24 at 10:08 +0800, erich wrote:
> Dear Arjan van de Ven,
>
> I would keep dma_alloc_coherent usage.
>
> > [Exception is that you can say that you are ok with a bigger mask for
> > this type of memory, but just don't do that if you're not]
>
> Should I remove "pci_set_dma_mask(pci_device, DMA_64BIT_MASK)" for this
> case?

no what you have is correct; the function to change this behavior is
called
pci_set_consistent_dma_mask()

pci_set_dma_mask() sets the mask for the "data" (eg dynamic mappings via
pci_map_single and pci_map_page and such). pci_set_consistent_dma_mask()
sets the mask for all the "consistent" allocators.

so again your code is fine as is. If you want to explicitly set that
mask to DMA_32BIT_MASK as documentation that you REALLY want it to be 32
bit, that is probably fine too, but not really needed.

Greetings,
Arjan van de Ven

2006-02-24 16:56:54

by Christoph Hellwig

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Thu, Feb 23, 2006 at 11:59:50AM +0000, Alan Cox wrote:
> On Iau, 2006-02-23 at 17:50 +0800, erich wrote:
> > But unfortunately I found some mainboards will hang up if I always enable
> > this function in my lab.
> > To avoid this issue, I do an option for this case.
> >
> > But Christoph Hellwig give me comment with it.
>
>
> Another thing you can also do for many of these cases is to use either
> the PCI or DMI interfaces to identify the problem board and
> automatically set the option as well.
>
> There are two ways to do this. One is

Please avoid that unless really nessecary. I doubt there's boards where
MSI would only be broken with the areca card but not with other MSI-capable
ones. If a board or chipset is generally broken vs MSI it should be
added to the global MSI blacklist. It's probably be nice to have a global
nomsi boot option instead of one in every driver aswell..

2006-02-24 17:03:49

by Randy Dunlap

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Fri, 24 Feb 2006, Christoph Hellwig wrote:

> On Thu, Feb 23, 2006 at 11:59:50AM +0000, Alan Cox wrote:
> > On Iau, 2006-02-23 at 17:50 +0800, erich wrote:
> > > But unfortunately I found some mainboards will hang up if I always enable
> > > this function in my lab.
> > > To avoid this issue, I do an option for this case.
> > >
> > > But Christoph Hellwig give me comment with it.
> >
> >
> > Another thing you can also do for many of these cases is to use either
> > the PCI or DMI interfaces to identify the problem board and
> > automatically set the option as well.
> >
> > There are two ways to do this. One is
>
> Please avoid that unless really nessecary. I doubt there's boards where
> MSI would only be broken with the areca card but not with other MSI-capable
> ones. If a board or chipset is generally broken vs MSI it should be
> added to the global MSI blacklist. It's probably be nice to have a global
> nomsi boot option instead of one in every driver aswell..

Jeff G. added an "msi" option to the sata_mv driver recently.
But yes, I expect it to be more of a platform issue than a
driver issue.

s2io network people report MSI interrupt problems on various
platforms (on netdev mailing list).

There are other reports (that I have at home, not visible to me
now). It would be good to have an MSI expert around.

http://www.xenotime.net/linux/patches/pci_nomsi.patch adds a
global boot option to disable MSI interrupt assignments.

--
~Randy

2006-02-24 19:38:32

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Fri, Feb 24, 2006 at 09:03:47AM -0800, Randy.Dunlap wrote:
> On Fri, 24 Feb 2006, Christoph Hellwig wrote:
> > Please avoid that unless really nessecary. I doubt there's boards where
> > MSI would only be broken with the areca card but not with other MSI-capable
> > ones. If a board or chipset is generally broken vs MSI it should be
> > added to the global MSI blacklist. It's probably be nice to have a global
> > nomsi boot option instead of one in every driver aswell..
>
> http://www.xenotime.net/linux/patches/pci_nomsi.patch adds a
> global boot option to disable MSI interrupt assignments.

I like it. How about adding pci=nomsi instead of pci_nomsi?

--- drivers/pci/pci.c 4 Feb 2006 04:51:55 -0000 1.28
+++ drivers/pci/pci.c 24 Feb 2006 19:33:25 -0000
@@ -900,8 +900,12 @@ static int __devinit pci_setup(char *str
if (k)
*k++ = 0;
if (*str && (str = pcibios_setup(str)) && *str) {
- /* PCI layer options should be handled here */
- printk(KERN_ERR "PCI: Unknown option `%s'\n", str);
+ if (!strcmp(str, "nomsi")) {
+ pci_msi_enable = 0;
+ } else {
+ printk(KERN_ERR "PCI: Unknown option `%s'\n",
+ str);
+ }
}
str = k;
}

2006-02-24 20:15:06

by Randy Dunlap

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Fri, 24 Feb 2006, Matthew Wilcox wrote:

> On Fri, Feb 24, 2006 at 09:03:47AM -0800, Randy.Dunlap wrote:
> > On Fri, 24 Feb 2006, Christoph Hellwig wrote:
> > > Please avoid that unless really nessecary. I doubt there's boards where
> > > MSI would only be broken with the areca card but not with other MSI-capable
> > > ones. If a board or chipset is generally broken vs MSI it should be
> > > added to the global MSI blacklist. It's probably be nice to have a global
> > > nomsi boot option instead of one in every driver aswell..
> >
> > http://www.xenotime.net/linux/patches/pci_nomsi.patch adds a
> > global boot option to disable MSI interrupt assignments.
>
> I like it. How about adding pci=nomsi instead of pci_nomsi?

Yes, that's probably better. Thanks for the change.

> --- drivers/pci/pci.c 4 Feb 2006 04:51:55 -0000 1.28
> +++ drivers/pci/pci.c 24 Feb 2006 19:33:25 -0000
> @@ -900,8 +900,12 @@ static int __devinit pci_setup(char *str
> if (k)
> *k++ = 0;
> if (*str && (str = pcibios_setup(str)) && *str) {
> - /* PCI layer options should be handled here */
> - printk(KERN_ERR "PCI: Unknown option `%s'\n", str);
> + if (!strcmp(str, "nomsi")) {
> + pci_msi_enable = 0;
> + } else {
> + printk(KERN_ERR "PCI: Unknown option `%s'\n",
> + str);
> + }
> }
> str = k;
> }
>
>

--
~Randy

2006-02-26 06:39:59

by Randy Dunlap

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Fri, 24 Feb 2006 12:38:30 -0700 Matthew Wilcox wrote:

> On Fri, Feb 24, 2006 at 09:03:47AM -0800, Randy.Dunlap wrote:
> > On Fri, 24 Feb 2006, Christoph Hellwig wrote:
> > > Please avoid that unless really nessecary. I doubt there's boards where
> > > MSI would only be broken with the areca card but not with other MSI-capable
> > > ones. If a board or chipset is generally broken vs MSI it should be
> > > added to the global MSI blacklist. It's probably be nice to have a global
> > > nomsi boot option instead of one in every driver aswell..
> >
> > http://www.xenotime.net/linux/patches/pci_nomsi.patch adds a
> > global boot option to disable MSI interrupt assignments.
>
> I like it. How about adding pci=nomsi instead of pci_nomsi?
>
> --- drivers/pci/pci.c 4 Feb 2006 04:51:55 -0000 1.28
> +++ drivers/pci/pci.c 24 Feb 2006 19:33:25 -0000
> @@ -900,8 +900,12 @@ static int __devinit pci_setup(char *str
> if (k)
> *k++ = 0;
> if (*str && (str = pcibios_setup(str)) && *str) {
> - /* PCI layer options should be handled here */
> - printk(KERN_ERR "PCI: Unknown option `%s'\n", str);
> + if (!strcmp(str, "nomsi")) {
> + pci_msi_enable = 0;
> + } else {
> + printk(KERN_ERR "PCI: Unknown option `%s'\n",
> + str);
> + }
> }
> str = k;

OK, updated patch for "pci=nomsi" is now at
http://www.xenotime.net/linux/patches/pci_nomsi.patch

Thanks.
---
~Randy

2006-02-26 16:02:50

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Sat, Feb 25, 2006 at 10:41:12PM -0800, Randy.Dunlap wrote:
> OK, updated patch for "pci=nomsi" is now at
> http://www.xenotime.net/linux/patches/pci_nomsi.patch

After sleeping on it, I realised this wouldn't work if CONFIG_PCI_MSI
is disabled. So how about this (not even compile-tested):

Signed-off-by: Matthew Wilcox <[email protected]>

Index: ./Documentation/kernel-parameters.txt
===================================================================
RCS file: /var/cvs/linux-2.6/Documentation/kernel-parameters.txt,v
retrieving revision 1.41.4.1
diff -u -p -r1.41.4.1 kernel-parameters.txt
--- ./Documentation/kernel-parameters.txt 18 Feb 2006 05:26:01 -0000 1.41.4.1
+++ ./Documentation/kernel-parameters.txt 26 Feb 2006 15:58:40 -0000
@@ -49,6 +49,7 @@ restrictions referred to are that the re
MCA MCA bus support is enabled.
MDA MDA console support is enabled.
MOUSE Appropriate mouse support is enabled.
+ MSI Message Signaled Interrupts (PCI).
MTD MTD support is enabled.
NET Appropriate network support is enabled.
NUMA NUMA support is enabled.
@@ -1135,6 +1136,9 @@ running once the system is up.
Mechanism 2.
nommconf [IA-32,X86_64] Disable use of MMCONFIG for PCI
Configuration
+ nomsi [MSI] If the PCI_MSI kernel config parameter is
+ enabled, this kernel boot option can be used to
+ disable the use of MSI interrupts system-wide.
nosort [IA-32] Don't sort PCI devices according to
order given by the PCI BIOS. This sorting is
done to get a device order compatible with
Index: ./drivers/pci/Kconfig
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/Kconfig,v
retrieving revision 1.8
diff -u -p -r1.8 Kconfig
--- ./drivers/pci/Kconfig 14 Sep 2005 12:56:32 -0000 1.8
+++ ./drivers/pci/Kconfig 26 Feb 2006 15:58:40 -0000
@@ -11,6 +11,10 @@ config PCI_MSI
generate an interrupt using an inbound Memory Write on its
PCI bus instead of asserting a device IRQ pin.

+ Use of PCI MSI interrupts can be disabled at kernel boot time
+ by using the 'pci=nomsi' option. This disables MSI for the
+ entire system.
+
If you don't know what to do here, say N.

config PCI_LEGACY_PROC
Index: ./drivers/pci/msi.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/msi.c,v
retrieving revision 1.17
diff -u -p -r1.17 msi.c
--- ./drivers/pci/msi.c 4 Feb 2006 04:51:55 -0000 1.17
+++ ./drivers/pci/msi.c 26 Feb 2006 15:58:40 -0000
@@ -755,6 +755,9 @@ void pci_disable_msi(struct pci_dev* dev
u16 control;
unsigned long flags;

+ if (!pci_msi_enable)
+ return;
+
if (!dev || !(pos = pci_find_capability(dev, PCI_CAP_ID_MSI)))
return;

@@ -1006,6 +1009,9 @@ void pci_disable_msix(struct pci_dev* de
int pos, temp;
u16 control;

+ if (!pci_msi_enable)
+ return;
+
if (!dev || !(pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)))
return;

@@ -1121,6 +1127,11 @@ void msi_remove_pci_irq_vectors(struct p
}
dev->irq = temp; /* Restore IOAPIC IRQ */
}
+}
+
+void pci_no_msi(void)
+{
+ pci_msi_enable = 0;
}

EXPORT_SYMBOL(pci_enable_msi);
Index: ./drivers/pci/pci.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/pci.c,v
retrieving revision 1.28
diff -u -p -r1.28 pci.c
--- ./drivers/pci/pci.c 4 Feb 2006 04:51:55 -0000 1.28
+++ ./drivers/pci/pci.c 26 Feb 2006 15:58:40 -0000
@@ -900,8 +900,12 @@ static int __devinit pci_setup(char *str
if (k)
*k++ = 0;
if (*str && (str = pcibios_setup(str)) && *str) {
- /* PCI layer options should be handled here */
- printk(KERN_ERR "PCI: Unknown option `%s'\n", str);
+ if (!strcmp(str, "nomsi")) {
+ pci_no_msi();
+ } else {
+ printk(KERN_ERR "PCI: Unknown option `%s'\n",
+ str);
+ }
}
str = k;
}
Index: ./drivers/pci/pci.h
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/pci.h,v
retrieving revision 1.13
diff -u -p -r1.13 pci.h
--- ./drivers/pci/pci.h 17 Jan 2006 14:51:41 -0000 1.13
+++ ./drivers/pci/pci.h 26 Feb 2006 15:58:40 -0000
@@ -50,8 +50,10 @@ extern int pci_msi_quirk;

#ifdef CONFIG_PCI_MSI
void disable_msi_mode(struct pci_dev *dev, int pos, int type);
+void pci_no_msi(void);
#else
static inline void disable_msi_mode(struct pci_dev *dev, int pos, int type) { }
+static inline void pci_no_msi(void) { }
#endif

extern int pcie_mch_quirk;

2006-02-26 18:59:11

by Randy Dunlap

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Sun, 26 Feb 2006 09:02:47 -0700 Matthew Wilcox wrote:

> On Sat, Feb 25, 2006 at 10:41:12PM -0800, Randy.Dunlap wrote:
> > OK, updated patch for "pci=nomsi" is now at
> > http://www.xenotime.net/linux/patches/pci_nomsi.patch
>
> After sleeping on it, I realised this wouldn't work if CONFIG_PCI_MSI
> is disabled. So how about this (not even compile-tested):

It's good. Thanks for catching that.

> Signed-off-by: Matthew Wilcox <[email protected]>
Signed-off-by: Randy Dunlap <[email protected]>


> Index: ./Documentation/kernel-parameters.txt
> ===================================================================
> RCS file: /var/cvs/linux-2.6/Documentation/kernel-parameters.txt,v
> retrieving revision 1.41.4.1
> diff -u -p -r1.41.4.1 kernel-parameters.txt
> --- ./Documentation/kernel-parameters.txt 18 Feb 2006 05:26:01 -0000 1.41.4.1
> +++ ./Documentation/kernel-parameters.txt 26 Feb 2006 15:58:40 -0000
> @@ -49,6 +49,7 @@ restrictions referred to are that the re
> MCA MCA bus support is enabled.
> MDA MDA console support is enabled.
> MOUSE Appropriate mouse support is enabled.
> + MSI Message Signaled Interrupts (PCI).
> MTD MTD support is enabled.
> NET Appropriate network support is enabled.
> NUMA NUMA support is enabled.
> @@ -1135,6 +1136,9 @@ running once the system is up.
> Mechanism 2.
> nommconf [IA-32,X86_64] Disable use of MMCONFIG for PCI
> Configuration
> + nomsi [MSI] If the PCI_MSI kernel config parameter is
> + enabled, this kernel boot option can be used to
> + disable the use of MSI interrupts system-wide.
> nosort [IA-32] Don't sort PCI devices according to
> order given by the PCI BIOS. This sorting is
> done to get a device order compatible with
> Index: ./drivers/pci/Kconfig
> ===================================================================
> RCS file: /var/cvs/linux-2.6/drivers/pci/Kconfig,v
> retrieving revision 1.8
> diff -u -p -r1.8 Kconfig
> --- ./drivers/pci/Kconfig 14 Sep 2005 12:56:32 -0000 1.8
> +++ ./drivers/pci/Kconfig 26 Feb 2006 15:58:40 -0000
> @@ -11,6 +11,10 @@ config PCI_MSI
> generate an interrupt using an inbound Memory Write on its
> PCI bus instead of asserting a device IRQ pin.
>
> + Use of PCI MSI interrupts can be disabled at kernel boot time
> + by using the 'pci=nomsi' option. This disables MSI for the
> + entire system.
> +
> If you don't know what to do here, say N.
>
> config PCI_LEGACY_PROC
> Index: ./drivers/pci/msi.c
> ===================================================================
> RCS file: /var/cvs/linux-2.6/drivers/pci/msi.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 msi.c
> --- ./drivers/pci/msi.c 4 Feb 2006 04:51:55 -0000 1.17
> +++ ./drivers/pci/msi.c 26 Feb 2006 15:58:40 -0000
> @@ -755,6 +755,9 @@ void pci_disable_msi(struct pci_dev* dev
> u16 control;
> unsigned long flags;
>
> + if (!pci_msi_enable)
> + return;
> +
> if (!dev || !(pos = pci_find_capability(dev, PCI_CAP_ID_MSI)))
> return;
>
> @@ -1006,6 +1009,9 @@ void pci_disable_msix(struct pci_dev* de
> int pos, temp;
> u16 control;
>
> + if (!pci_msi_enable)
> + return;
> +
> if (!dev || !(pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)))
> return;
>
> @@ -1121,6 +1127,11 @@ void msi_remove_pci_irq_vectors(struct p
> }
> dev->irq = temp; /* Restore IOAPIC IRQ */
> }
> +}
> +
> +void pci_no_msi(void)
> +{
> + pci_msi_enable = 0;
> }
>
> EXPORT_SYMBOL(pci_enable_msi);
> Index: ./drivers/pci/pci.c
> ===================================================================
> RCS file: /var/cvs/linux-2.6/drivers/pci/pci.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 pci.c
> --- ./drivers/pci/pci.c 4 Feb 2006 04:51:55 -0000 1.28
> +++ ./drivers/pci/pci.c 26 Feb 2006 15:58:40 -0000
> @@ -900,8 +900,12 @@ static int __devinit pci_setup(char *str
> if (k)
> *k++ = 0;
> if (*str && (str = pcibios_setup(str)) && *str) {
> - /* PCI layer options should be handled here */
> - printk(KERN_ERR "PCI: Unknown option `%s'\n", str);
> + if (!strcmp(str, "nomsi")) {
> + pci_no_msi();
> + } else {
> + printk(KERN_ERR "PCI: Unknown option `%s'\n",
> + str);
> + }
> }
> str = k;
> }
> Index: ./drivers/pci/pci.h
> ===================================================================
> RCS file: /var/cvs/linux-2.6/drivers/pci/pci.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 pci.h
> --- ./drivers/pci/pci.h 17 Jan 2006 14:51:41 -0000 1.13
> +++ ./drivers/pci/pci.h 26 Feb 2006 15:58:40 -0000
> @@ -50,8 +50,10 @@ extern int pci_msi_quirk;
>
> #ifdef CONFIG_PCI_MSI
> void disable_msi_mode(struct pci_dev *dev, int pos, int type);
> +void pci_no_msi(void);
> #else
> static inline void disable_msi_mode(struct pci_dev *dev, int pos, int type) { }
> +static inline void pci_no_msi(void) { }
> #endif
>
> extern int pcie_mch_quirk;
>


---

2006-02-27 11:27:49

by erich

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

Dear Christoph Hellwig,

Do you have any comments with arcmsr SATA RAID driver on sysfs attribute?
There were four types of function template completed in linux.
iscsi_function_template
sas_function_template
spi_function_template
fc_function_template
Do you have opintion with "arcmsr_transport_functions" ?
and Which function templete does "arcmsr" belong to?

Best Regards
Erich Chen

2006-02-27 12:37:03

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Areca RAID driver remaining items?

On Mon, Feb 27, 2006 at 07:27:33PM +0800, erich wrote:
> Dear Christoph Hellwig,
>
> Do you have any comments with arcmsr SATA RAID driver on sysfs attribute?
> There were four types of function template completed in linux.
> iscsi_function_template
> sas_function_template
> spi_function_template
> fc_function_template
> Do you have opintion with "arcmsr_transport_functions" ?
> and Which function templete does "arcmsr" belong to?

The transport really refers to the physical cabling. iSCSI is
SCSI-over-IP, SAS is Serial Attached SCSI, SPI is Parallel SCSI and FC
is Fibre Channel. It seems to me from your website that you're
using SATA-II drives, so you'll want to look at the SAS template for
exposing cabling details.

You missed one useful class though, the raid_function_template, which
you almost certainly want to use. See drivers/scsi/raid_class.c and
include/linux/raid_class.h. It's early days for the RAID class, so you
may wish to extend it to meet your needs.

James, I presume it's been mis-placed for convenience and it'll move to
block/ or drivers/block/ at some point?