* Re: [RFC Patch V1 22/30] mm, of: Use cpu_to_mem()/numa_mem_id() to support memoryless node
From: Nishanth Aravamudan @ 2014-07-28 19:26 UTC (permalink / raw)
To: Grant Likely
Cc: Jiang Liu, Andrew Morton, Mel Gorman, David Rientjes,
Mike Galbraith, Peter Zijlstra, Rafael J . Wysocki, Rob Herring,
Tony Luck, linux-mm, linux-hotplug, linux-kernel, devicetree
In-Reply-To: <20140728133040.854F5C4095E@trevor.secretlab.ca>
On 28.07.2014 [07:30:40 -0600], Grant Likely wrote:
> On Mon, 21 Jul 2014 10:52:41 -0700, Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
> > On 11.07.2014 [15:37:39 +0800], Jiang Liu wrote:
> > > When CONFIG_HAVE_MEMORYLESS_NODES is enabled, cpu_to_node()/numa_node_id()
> > > may return a node without memory, and later cause system failure/panic
> > > when calling kmalloc_node() and friends with returned node id.
> > > So use cpu_to_mem()/numa_mem_id() instead to get the nearest node with
> > > memory for the/current cpu.
> > >
> > > If CONFIG_HAVE_MEMORYLESS_NODES is disabled, cpu_to_mem()/numa_mem_id()
> > > is the same as cpu_to_node()/numa_node_id().
> > >
> > > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> > > ---
> > > drivers/of/base.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > > index b9864806e9b8..40d4772973ad 100644
> > > --- a/drivers/of/base.c
> > > +++ b/drivers/of/base.c
> > > @@ -85,7 +85,7 @@ EXPORT_SYMBOL(of_n_size_cells);
> > > #ifdef CONFIG_NUMA
> > > int __weak of_node_to_nid(struct device_node *np)
> > > {
> > > - return numa_node_id();
> > > + return numa_mem_id();
> > > }
> > > #endif
> >
> > Um, NAK. of_node_to_nid() returns the NUMA node ID for a given device
> > tree node. The default should be the physically local NUMA node, not the
> > nearest memory-containing node.
>
> That description doesn't match the code. This patch only changes the
> default implementation of of_node_to_nid() which doesn't take the device
> node into account *at all* when returning a node ID. Just look at the
> diff.
I meant that of_node_to_nid() seems to be used throughout the call-sites
to indicate caller locality. We want to keep using cpu_to_node() there,
and fallback appropriately in the MM (when allocations occur offnode due
to memoryless nodes), not indicate memory-specific topology the caller
itself. There was a long thread between between Tejun and I that
discussed what we are trying for: https://lkml.org/lkml/2014/7/18/278
I understand that the code unconditionally returns current's NUMA node
ID right now (ignoring the device node). That seems correct, to me, for
something like:
of_device_add:
/* device_add will assume that this device is on the same node as
* the parent. If there is no parent defined, set the node
* explicitly */
if (!ofdev->dev.parent)
set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
I don't think we want the default implementation to set the NUMA node of
a dev to the nearest NUMA node with memory?
> I think this patch is correct, and it doesn't affect the override
> versions provided by powerpc and sparc.
Yes, agreed, so maybe it doesn't matter. I guess my point was simply
that it only seems reasonable to change callers of cpu_to_node() to
cpu_to_mem() that aren't in the core MM is if they care about memoryless
nodes explicitly. I don't think the OF code does, so I don't think it
should change.
Sorry for my premature NAK and lack of clarity in my explanation.
-Nish
^ permalink raw reply
* Re: [Patch Part3 V4 00/21] Enable support of Intel DMAR device hotplug
From: Joerg Roedel @ 2014-07-29 10:49 UTC (permalink / raw)
To: Jiang Liu
Cc: David Woodhouse, Yinghai Lu, Bjorn Helgaas, Dan Williams,
Vinod Koul, Rafael J . Wysocki, Ashok Raj, Yijing Wang, Tony Luck,
iommu, linux-pci, linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <1405059585-10620-1-git-send-email-jiang.liu@linux.intel.com>
On Fri, Jul 11, 2014 at 02:19:24PM +0800, Jiang Liu wrote:
> Patch 1-13 are bugfixes and code improvement for current drivers.
Okay, I applied these for now (1-13 only) so that you don't have to
rebase everything next time.
> Patch 14-17 enhances DMAR framework to support hotplug
> Patch 18 enhances Intel interrupt remapping driver to support hotplug
> Patch 19 enhances error handling in Intel IR driver
> Patch 20 enhance Intel IOMMU to support hotplug
> Patch 21 enhance ACPI pci_root driver to handle DMAR units
For these I'd like to see some reviews/acked-bys/tested-bys so that we
gain more confidence on them.
Joerg
^ permalink raw reply
* Re: [Patch Part3 V4 00/21] Enable support of Intel DMAR device hotplug
From: Jiang Liu @ 2014-07-29 12:09 UTC (permalink / raw)
To: Joerg Roedel
Cc: David Woodhouse, Yinghai Lu, Bjorn Helgaas, Dan Williams,
Vinod Koul, Rafael J . Wysocki, Ashok Raj, Yijing Wang, Tony Luck,
iommu, linux-pci, linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <20140729104932.GC9809@8bytes.org>
Thanks Joerg, will resend remaining patches after 3.17 merging window.
On 2014/7/29 18:49, Joerg Roedel wrote:
> On Fri, Jul 11, 2014 at 02:19:24PM +0800, Jiang Liu wrote:
>> Patch 1-13 are bugfixes and code improvement for current drivers.
>
> Okay, I applied these for now (1-13 only) so that you don't have to
> rebase everything next time.
>
>> Patch 14-17 enhances DMAR framework to support hotplug
>> Patch 18 enhances Intel interrupt remapping driver to support hotplug
>> Patch 19 enhances error handling in Intel IR driver
>> Patch 20 enhance Intel IOMMU to support hotplug
>> Patch 21 enhance ACPI pci_root driver to handle DMAR units
>
> For these I'd like to see some reviews/acked-bys/tested-bys so that we
> gain more confidence on them.
>
>
> Joerg
>
^ permalink raw reply
* [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Aniroop Mathur @ 2014-08-01 17:25 UTC (permalink / raw)
To: linux-kernel, linux-hotplug, gregkh
Dear Mr. Greg Kroah-Hartman and Linux Community,
Greetings of the day !! :)
I am Aniroop Mathur working on Linux Kernel for last two years.
I am stuck at one point and could not find the solution over internet.
I posted on linuxquestions.org too.
So I need your help and suggestion for it.
Can you please help in answering my query as below:
==========================In function device_add of /drivers/base/core.c file, it is mentioned:
/*
* for statically allocated devices, which should all be converted
* some day, we need to initialize the name. We prevent reading back
* the name, and force the use of dev_name()
*/
if (dev->init_name) {
dev_set_name(dev, "%s", dev->init_name);
dev->init_name = NULL;
}
Except forcing the use of dev_name to read device name,
Is there any other reason to make init_name as NULL ?
And if it is not made NULL, is there any problem or side-effect ?
===========================
Link at linuxquestions.org:
http://www.linuxquestions.org/questions/linux-kernel-70/why-dev-init_name-%3D-null-in-device_add-function-4175504749/
Thanks a lot in advance !
Best Regards,
Aniroop Mathur
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Greg KH @ 2014-08-01 19:23 UTC (permalink / raw)
To: Aniroop Mathur; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <CADYu30_FciG_aTTV4DFNdy+CdRChMYQ7pAGZGambE1F9s=ofYQ@mail.gmail.com>
On Fri, Aug 01, 2014 at 10:43:23PM +0530, Aniroop Mathur wrote:
> Dear Mr. Greg Kroah-Hartman and Linux Community,
> Greetings of the day !! :)
>
> I am Aniroop Mathur working on Linux Kernel for last two years.
> I am stuck at one point and could not find the solution over internet.
> I posted on linuxquestions.org too.
> So I need your help and suggestion for it.
>
> Can you please help in answering my query as below:
>
> ==========================> In function device_add of /drivers/base/core.c file, it is mentioned:
> /*
> * for statically allocated devices, which should all be converted
> * some day, we need to initialize the name. We prevent reading back
> * the name, and force the use of dev_name()
> */
> if (dev->init_name) {
> dev_set_name(dev, "%s", dev->init_name);
> dev->init_name = NULL;
> }
>
>
> Except forcing the use of dev_name to read device name,
> Is there any other reason to make init_name as NULL ?
Why would you want init_name to not be NULL?
> And if it is not made NULL, is there any problem or side-effect ?
Yes, people would start to use it thinking it was the real name of the
device, when it might not be.
greg k-h
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Aniroop Mathur @ 2014-08-01 22:36 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <20140801192300.GB16085@kroah.com>
On Sat, Aug 2, 2014 at 12:53 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Aug 01, 2014 at 10:43:23PM +0530, Aniroop Mathur wrote:
>> Dear Mr. Greg Kroah-Hartman and Linux Community,
>> Greetings of the day !! :)
>>
>> I am Aniroop Mathur working on Linux Kernel for last two years.
>> I am stuck at one point and could not find the solution over internet.
>> I posted on linuxquestions.org too.
>> So I need your help and suggestion for it.
>>
>> Can you please help in answering my query as below:
>>
>> ==========================>> In function device_add of /drivers/base/core.c file, it is mentioned:
>> /*
>> * for statically allocated devices, which should all be converted
>> * some day, we need to initialize the name. We prevent reading back
>> * the name, and force the use of dev_name()
>> */
>> if (dev->init_name) {
>> dev_set_name(dev, "%s", dev->init_name);
>> dev->init_name = NULL;
>> }
>>
>>
>> Except forcing the use of dev_name to read device name,
>> Is there any other reason to make init_name as NULL ?
>
> Why would you want init_name to not be NULL?
>
Currently in kernel, we cannot set name of event node.
If dev->init_name is not set as NULL in device_add(),
then I can easily set name of event node in evdev_dev.c
file as below:
if(dev->init_name) {
sprintf(dev->init_name, "event_%s", dev->init_name);
}
error = device_add(&evdev->dev);
And in some input device driver code, I will use like below:
dev->init_name = "accelerometer";
input_register_device(dev);
So, overall output will be
/dev/input/event<x> --> /dev/input/event_accelerometer
sys/class/input/input<x> --> sys/class/input/accelerometer
In short, input and event node names are set just by
adding one line, which i found quite efficient.
There is other way also to set name of event node but
it involves using extra variable and little more code,
So I am looking for best solution possible. :)
>> And if it is not made NULL, is there any problem or side-effect ?
>
> Yes, people would start to use it thinking it was the real name of the
> device, when it might not be.
>
As the name itself nicely suggests, it is just a initial name.
It may not be the final name as developer can change the name
using dev_set_name. Also dev_name api is available to get
final/current name of the device.
Thanks and Regards,
Aniroop Mathur
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Greg KH @ 2014-08-01 22:41 UTC (permalink / raw)
To: Aniroop Mathur; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <CADYu30_o7rzERKEDe=eZiONp6n1ktMWPydMnXQjDeLrDFQh+vg@mail.gmail.com>
On Sat, Aug 02, 2014 at 03:54:32AM +0530, Aniroop Mathur wrote:
> On Sat, Aug 2, 2014 at 12:53 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Aug 01, 2014 at 10:43:23PM +0530, Aniroop Mathur wrote:
> >> Dear Mr. Greg Kroah-Hartman and Linux Community,
> >> Greetings of the day !! :)
> >>
> >> I am Aniroop Mathur working on Linux Kernel for last two years.
> >> I am stuck at one point and could not find the solution over internet.
> >> I posted on linuxquestions.org too.
> >> So I need your help and suggestion for it.
> >>
> >> Can you please help in answering my query as below:
> >>
> >> ==========================> >> In function device_add of /drivers/base/core.c file, it is mentioned:
> >> /*
> >> * for statically allocated devices, which should all be converted
> >> * some day, we need to initialize the name. We prevent reading back
> >> * the name, and force the use of dev_name()
> >> */
> >> if (dev->init_name) {
> >> dev_set_name(dev, "%s", dev->init_name);
> >> dev->init_name = NULL;
> >> }
> >>
> >>
> >> Except forcing the use of dev_name to read device name,
> >> Is there any other reason to make init_name as NULL ?
> >
> > Why would you want init_name to not be NULL?
> >
>
> Currently in kernel, we cannot set name of event node.
What do you mean by "event node"?
> If dev->init_name is not set as NULL in device_add(),
> then I can easily set name of event node in evdev_dev.c
> file as below:
>
> if(dev->init_name) {
> sprintf(dev->init_name, "event_%s", dev->init_name);
> }
> error = device_add(&evdev->dev);
>
> And in some input device driver code, I will use like below:
> dev->init_name = "accelerometer";
> input_register_device(dev);
What's wrong with:
dev_set_name(dev, "%s", "accelerometer");
input_register_device(dev);
> So, overall output will be
> /dev/input/event<x> --> /dev/input/event_accelerometer
> sys/class/input/input<x> --> sys/class/input/accelerometer
>
> In short, input and event node names are set just by
> adding one line, which i found quite efficient.
> There is other way also to set name of event node but
> it involves using extra variable and little more code,
> So I am looking for best solution possible. :)
Only use init_name for static struct devices, for a dynamic one, jsut
set the name like everyone else does, how is that "more" code than
anything else?
> >> And if it is not made NULL, is there any problem or side-effect ?
> >
> > Yes, people would start to use it thinking it was the real name of the
> > device, when it might not be.
> >
>
> As the name itself nicely suggests, it is just a initial name.
So please do not use it, someday it will go away...
thanks,
greg k-h
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Aniroop Mathur @ 2014-08-01 23:56 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <20140801224127.GA11596@kroah.com>
On Sat, Aug 2, 2014 at 4:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Aug 02, 2014 at 03:54:32AM +0530, Aniroop Mathur wrote:
>> On Sat, Aug 2, 2014 at 12:53 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Fri, Aug 01, 2014 at 10:43:23PM +0530, Aniroop Mathur wrote:
>> >> Dear Mr. Greg Kroah-Hartman and Linux Community,
>> >> Greetings of the day !! :)
>> >>
>> >> I am Aniroop Mathur working on Linux Kernel for last two years.
>> >> I am stuck at one point and could not find the solution over internet.
>> >> I posted on linuxquestions.org too.
>> >> So I need your help and suggestion for it.
>> >>
>> >> Can you please help in answering my query as below:
>> >>
>> >> ==========================>> >> In function device_add of /drivers/base/core.c file, it is mentioned:
>> >> /*
>> >> * for statically allocated devices, which should all be converted
>> >> * some day, we need to initialize the name. We prevent reading back
>> >> * the name, and force the use of dev_name()
>> >> */
>> >> if (dev->init_name) {
>> >> dev_set_name(dev, "%s", dev->init_name);
>> >> dev->init_name = NULL;
>> >> }
>> >>
>> >>
>> >> Except forcing the use of dev_name to read device name,
>> >> Is there any other reason to make init_name as NULL ?
>> >
>> > Why would you want init_name to not be NULL?
>> >
>>
>> Currently in kernel, we cannot set name of event node.
>
> What do you mean by "event node"?
>
I am referring to the device node which HAL/Application uses
as an interface to interact with kernel for read/write data operation.
like /dev/input/event0, /dev/input/event1, etc
int fd = open("/dev/input/event0", O_RDONLY);
int res = read(fd, &input_event, sizeof(input_event));
>> If dev->init_name is not set as NULL in device_add(),
>> then I can easily set name of event node in evdev_dev.c
>> file as below:
>>
>> if(dev->init_name) {
>> sprintf(dev->init_name, "event_%s", dev->init_name);
>> }
>> error = device_add(&evdev->dev);
>>
>> And in some input device driver code, I will use like below:
>> dev->init_name = "accelerometer";
>> input_register_device(dev);
>
> What's wrong with:
> dev_set_name(dev, "%s", "accelerometer");
> input_register_device(dev);
>
It is good way for setting name of "Input node".
But it will not set name of "Event node".
input_register_device call sets name of two nodes
1. Input node - sys/class/input/input<x>
2. Event node - /dev/input/event<x> or sys/class/input/event<x>
Here, input<x> and event<x> are default names set without using
dev_set_name in driver.
input<x> - kobject name of struct input_dev;
event<x> - kobject name of struct evdev;
In input_register_device(dev) function call,
dev refers to struct input_dev and not struct evdev.
struct input_dev is defined in input.h and
struct evdev is defined in evdev.c (not a header file).
So driver can use struct input_dev and set its name
using dev_set_name(dev, "%s", "accelerometer");
But struct evdev is not accessible to driver as it is defined in
evdev.c file. So driver cannot use dev_set_name for evdev.
It can only be used in evdev.c file.
So above method output will be
sys/class/input/accelerometer
/dev/input/event<x> and sys/class/input/event<x>
(event node name not set)
>> So, overall output will be
>> /dev/input/event<x> --> /dev/input/event_accelerometer
>> sys/class/input/input<x> --> sys/class/input/accelerometer
>>
>> In short, input and event node names are set just by
>> adding one line, which i found quite efficient.
>> There is other way also to set name of event node but
>> it involves using extra variable and little more code,
>> So I am looking for best solution possible. :)
>
> Only use init_name for static struct devices, for a dynamic one, jsut
> set the name like everyone else does, how is that "more" code than
> anything else?
>
Can you please elaborate what do you mean by static struct devices ?
Can I consider embedded accelerometer, proximity, gyro sensor,
touch panel in android mobile devices as static devices ?
Input subsystem is setting default name of input and event node.
This is the normal method everyone uses.
I do not want to use default name like event0, event4, input2, etc
My aim is to set name of input and event node through driver as desired.
So after discussion of other ways to set name through driver,
we can compare which way is better.
In one other method, there is a need to add extra variable in
struct input_dev and hence more memory size.
>> >> And if it is not made NULL, is there any problem or side-effect ?
>> >
>> > Yes, people would start to use it thinking it was the real name of the
>> > device, when it might not be.
>> >
>>
>> As the name itself nicely suggests, it is just a initial name.
>
> So please do not use it, someday it will go away...
>
> thanks,
>
Thanks and Regards,
Aniroop Mathur
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Greg KH @ 2014-08-02 2:39 UTC (permalink / raw)
To: Aniroop Mathur; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <CADYu309eydkp83a1aMybHn5vyLLGBAmFDYeR59RRiC+mGc9w7w@mail.gmail.com>
On Sat, Aug 02, 2014 at 05:14:44AM +0530, Aniroop Mathur wrote:
> >> So, overall output will be
> >> /dev/input/event<x> --> /dev/input/event_accelerometer
> >> sys/class/input/input<x> --> sys/class/input/accelerometer
> >>
> >> In short, input and event node names are set just by
> >> adding one line, which i found quite efficient.
> >> There is other way also to set name of event node but
> >> it involves using extra variable and little more code,
> >> So I am looking for best solution possible. :)
> >
> > Only use init_name for static struct devices, for a dynamic one, jsut
> > set the name like everyone else does, how is that "more" code than
> > anything else?
> >
>
> Can you please elaborate what do you mean by static struct devices ?
Exactly what you wrote there, 'static' is a C thing, right?
> Can I consider embedded accelerometer, proximity, gyro sensor,
> touch panel in android mobile devices as static devices ?
Nope, because in my Android phone, I can yank out those and add new ones
on the fly while the phone is running. The kernel is fully dynamic that
way.
> Input subsystem is setting default name of input and event node.
As it should.
> This is the normal method everyone uses.
And so should you :)
> I do not want to use default name like event0, event4, input2, etc
Yes, you really do.
> My aim is to set name of input and event node through driver as desired.
Nope, that's userspace's job, just use udev to rename the device node,
or even better yet, create a symlink like /dev/input/by-id/ has if you
really need more than that.
Don't mess with kernel device names, we can't do that without breaking
tons of things, there's a reason that we standardized on something,
please use it.
good luck,
greg k-h
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Aniroop Mathur @ 2014-08-03 16:25 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <CADYu309eydkp83a1aMybHn5vyLLGBAmFDYeR59RRiC+mGc9w7w@mail.gmail.com>
Dear Mr. Greg KH and Linux Kernel Community,
Greetings of the day ! :)
I need to mark one open linux thread as solved as it is open for a
long time now.
Kindly help in answering my below two queries. :)
On Sat, Aug 2, 2014 at 5:14 AM, Aniroop Mathur <aniroop.mathur@gmail.com> wrote:
> On Sat, Aug 2, 2014 at 4:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Sat, Aug 02, 2014 at 03:54:32AM +0530, Aniroop Mathur wrote:
>>> On Sat, Aug 2, 2014 at 12:53 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>>> > On Fri, Aug 01, 2014 at 10:43:23PM +0530, Aniroop Mathur wrote:
>>> >> Dear Mr. Greg Kroah-Hartman and Linux Community,
>>> >> Greetings of the day !! :)
>>> >>
>>> >> I am Aniroop Mathur working on Linux Kernel for last two years.
>>> >> I am stuck at one point and could not find the solution over internet.
>>> >> I posted on linuxquestions.org too.
>>> >> So I need your help and suggestion for it.
>>> >>
>>> >> Can you please help in answering my query as below:
>>> >>
>>> >> ==========================>>> >> In function device_add of /drivers/base/core.c file, it is mentioned:
>>> >> /*
>>> >> * for statically allocated devices, which should all be converted
>>> >> * some day, we need to initialize the name. We prevent reading back
>>> >> * the name, and force the use of dev_name()
>>> >> */
>>> >> if (dev->init_name) {
>>> >> dev_set_name(dev, "%s", dev->init_name);
>>> >> dev->init_name = NULL;
>>> >> }
>>> >>
>>> >>
>>> >> Except forcing the use of dev_name to read device name,
>>> >> Is there any other reason to make init_name as NULL ?
>>> >
>>> > Why would you want init_name to not be NULL?
>>> >
>>>
>>> Currently in kernel, we cannot set name of event node.
>>
>> What do you mean by "event node"?
>>
>
> I am referring to the device node which HAL/Application uses
> as an interface to interact with kernel for read/write data operation.
> like /dev/input/event0, /dev/input/event1, etc
>
> int fd = open("/dev/input/event0", O_RDONLY);
> int res = read(fd, &input_event, sizeof(input_event));
>
>>> If dev->init_name is not set as NULL in device_add(),
>>> then I can easily set name of event node in evdev_dev.c
>>> file as below:
>>>
>>> if(dev->init_name) {
>>> sprintf(dev->init_name, "event_%s", dev->init_name);
>>> }
>>> error = device_add(&evdev->dev);
>>>
>>> And in some input device driver code, I will use like below:
>>> dev->init_name = "accelerometer";
>>> input_register_device(dev);
>>
>> What's wrong with:
>> dev_set_name(dev, "%s", "accelerometer");
>> input_register_device(dev);
>>
>
> It is good way for setting name of "Input node".
> But it will not set name of "Event node".
>
> input_register_device call sets name of two nodes
> 1. Input node - sys/class/input/input<x>
> 2. Event node - /dev/input/event<x> or sys/class/input/event<x>
>
> Here, input<x> and event<x> are default names set without using
> dev_set_name in driver.
> input<x> - kobject name of struct input_dev;
> event<x> - kobject name of struct evdev;
>
> In input_register_device(dev) function call,
> dev refers to struct input_dev and not struct evdev.
>
> struct input_dev is defined in input.h and
> struct evdev is defined in evdev.c (not a header file).
>
> So driver can use struct input_dev and set its name
> using dev_set_name(dev, "%s", "accelerometer");
> But struct evdev is not accessible to driver as it is defined in
> evdev.c file. So driver cannot use dev_set_name for evdev.
> It can only be used in evdev.c file.
>
> So above method output will be
> sys/class/input/accelerometer
> /dev/input/event<x> and sys/class/input/event<x>
> (event node name not set)
>
>>> So, overall output will be
>>> /dev/input/event<x> --> /dev/input/event_accelerometer
>>> sys/class/input/input<x> --> sys/class/input/accelerometer
>>>
>>> In short, input and event node names are set just by
>>> adding one line, which i found quite efficient.
>>> There is other way also to set name of event node but
>>> it involves using extra variable and little more code,
>>> So I am looking for best solution possible. :)
>>
>> Only use init_name for static struct devices, for a dynamic one, jsut
>> set the name like everyone else does, how is that "more" code than
>> anything else?
>>
>
> Can you please elaborate what do you mean by static struct devices ?
> Can I consider embedded accelerometer, proximity, gyro sensor,
> touch panel in android mobile devices as static devices ?
>
1. What are statically allocated devices and dynamically allocated devices ?
> Input subsystem is setting default name of input and event node.
> This is the normal method everyone uses.
>
> I do not want to use default name like event0, event4, input2, etc
> My aim is to set name of input and event node through driver as desired.
> So after discussion of other ways to set name through driver,
> we can compare which way is better.
> In one other method, there is a need to add extra variable in
> struct input_dev and hence more memory size.
>
>>> >> And if it is not made NULL, is there any problem or side-effect ?
>>> >
>>> > Yes, people would start to use it thinking it was the real name of the
>>> > device, when it might not be.
>>> >
>>>
>>> As the name itself nicely suggests, it is just a initial name.
>>
>> So please do not use it, someday it will go away...
>>
2. Why init_name field will be removed from struct device someday ?
>> thanks,
>>
>
Thanks,
Aniroop
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Aniroop Mathur @ 2014-08-03 18:54 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <20140802023958.GA22728@kroah.com>
On Sat, Aug 2, 2014 at 8:09 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Aug 02, 2014 at 05:14:44AM +0530, Aniroop Mathur wrote:
>> >> So, overall output will be
>> >> /dev/input/event<x> --> /dev/input/event_accelerometer
>> >> sys/class/input/input<x> --> sys/class/input/accelerometer
>> >>
>> >> In short, input and event node names are set just by
>> >> adding one line, which i found quite efficient.
>> >> There is other way also to set name of event node but
>> >> it involves using extra variable and little more code,
>> >> So I am looking for best solution possible. :)
>> >
>> > Only use init_name for static struct devices, for a dynamic one, jsut
>> > set the name like everyone else does, how is that "more" code than
>> > anything else?
>> >
>>
>> Can you please elaborate what do you mean by static struct devices ?
>
> Exactly what you wrote there, 'static' is a C thing, right?
>
>> Can I consider embedded accelerometer, proximity, gyro sensor,
>> touch panel in android mobile devices as static devices ?
>
> Nope, because in my Android phone, I can yank out those and add new ones
> on the fly while the phone is running. The kernel is fully dynamic that
> way.
>
>> Input subsystem is setting default name of input and event node.
>
> As it should.
>
>> This is the normal method everyone uses.
>
> And so should you :)
>
>> I do not want to use default name like event0, event4, input2, etc
>
> Yes, you really do.
>
>> My aim is to set name of input and event node through driver as desired.
>
> Nope, that's userspace's job, just use udev to rename the device node,
> or even better yet, create a symlink like /dev/input/by-id/ has if you
> really need more than that.
>
> Don't mess with kernel device names, we can't do that without breaking
> tons of things, there's a reason that we standardized on something,
> please use it.
>
Okay, will follow the same. :)
One last thing,
Why init_name will go away some day ?
Is it because now we have dev_set_name,
so there is no need of it anymore.
Moreover as you said,
"people would start to use it thinking it was the real name of the
device, when it might not be."
So, there remains no purpose of init_name anymore, right ?
> good luck,
>
> greg k-h
Thanks,
Aniroop
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Greg KH @ 2014-08-04 4:35 UTC (permalink / raw)
To: Aniroop Mathur; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <CADYu30_EN78+1CdD=VF8gi-5H2Ot7ArKWq5DQ5HaptwTuL3C6A@mail.gmail.com>
On Mon, Aug 04, 2014 at 12:24:06AM +0530, Aniroop Mathur wrote:
> On Sat, Aug 2, 2014 at 8:09 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Sat, Aug 02, 2014 at 05:14:44AM +0530, Aniroop Mathur wrote:
> >> >> So, overall output will be
> >> >> /dev/input/event<x> --> /dev/input/event_accelerometer
> >> >> sys/class/input/input<x> --> sys/class/input/accelerometer
> >> >>
> >> >> In short, input and event node names are set just by
> >> >> adding one line, which i found quite efficient.
> >> >> There is other way also to set name of event node but
> >> >> it involves using extra variable and little more code,
> >> >> So I am looking for best solution possible. :)
> >> >
> >> > Only use init_name for static struct devices, for a dynamic one, jsut
> >> > set the name like everyone else does, how is that "more" code than
> >> > anything else?
> >> >
> >>
> >> Can you please elaborate what do you mean by static struct devices ?
> >
> > Exactly what you wrote there, 'static' is a C thing, right?
> >
> >> Can I consider embedded accelerometer, proximity, gyro sensor,
> >> touch panel in android mobile devices as static devices ?
> >
> > Nope, because in my Android phone, I can yank out those and add new ones
> > on the fly while the phone is running. The kernel is fully dynamic that
> > way.
> >
> >> Input subsystem is setting default name of input and event node.
> >
> > As it should.
> >
> >> This is the normal method everyone uses.
> >
> > And so should you :)
> >
> >> I do not want to use default name like event0, event4, input2, etc
> >
> > Yes, you really do.
> >
> >> My aim is to set name of input and event node through driver as desired.
> >
> > Nope, that's userspace's job, just use udev to rename the device node,
> > or even better yet, create a symlink like /dev/input/by-id/ has if you
> > really need more than that.
> >
> > Don't mess with kernel device names, we can't do that without breaking
> > tons of things, there's a reason that we standardized on something,
> > please use it.
> >
>
> Okay, will follow the same. :)
>
> One last thing,
> Why init_name will go away some day ?
Because you should never have a static struct device in the kernel.
It's a reference counted object that needs to always be dynamically
allocated in order to properly work. Unfortunatly, there are some users
who have it allocated statically (on the stack) instead of dynamically
(on the heap). I'll get rid of them someday, it's _way_ down my list of
things to do...
thanks,
greg k-h
^ permalink raw reply
* Re: [Question: Drivers/base/core.c] Why dev->init_name = NULL in device_add function ?
From: Aniroop Mathur @ 2014-08-04 17:11 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hotplug
In-Reply-To: <20140804043541.GA4665@kroah.com>
On Mon, Aug 4, 2014 at 10:05 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Aug 04, 2014 at 12:24:06AM +0530, Aniroop Mathur wrote:
>> On Sat, Aug 2, 2014 at 8:09 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Sat, Aug 02, 2014 at 05:14:44AM +0530, Aniroop Mathur wrote:
>> >> >> So, overall output will be
>> >> >> /dev/input/event<x> --> /dev/input/event_accelerometer
>> >> >> sys/class/input/input<x> --> sys/class/input/accelerometer
>> >> >>
>> >> >> In short, input and event node names are set just by
>> >> >> adding one line, which i found quite efficient.
>> >> >> There is other way also to set name of event node but
>> >> >> it involves using extra variable and little more code,
>> >> >> So I am looking for best solution possible. :)
>> >> >
>> >> > Only use init_name for static struct devices, for a dynamic one, jsut
>> >> > set the name like everyone else does, how is that "more" code than
>> >> > anything else?
>> >> >
>> >>
>> >> Can you please elaborate what do you mean by static struct devices ?
>> >
>> > Exactly what you wrote there, 'static' is a C thing, right?
>> >
>> >> Can I consider embedded accelerometer, proximity, gyro sensor,
>> >> touch panel in android mobile devices as static devices ?
>> >
>> > Nope, because in my Android phone, I can yank out those and add new ones
>> > on the fly while the phone is running. The kernel is fully dynamic that
>> > way.
>> >
>> >> Input subsystem is setting default name of input and event node.
>> >
>> > As it should.
>> >
>> >> This is the normal method everyone uses.
>> >
>> > And so should you :)
>> >
>> >> I do not want to use default name like event0, event4, input2, etc
>> >
>> > Yes, you really do.
>> >
>> >> My aim is to set name of input and event node through driver as desired.
>> >
>> > Nope, that's userspace's job, just use udev to rename the device node,
>> > or even better yet, create a symlink like /dev/input/by-id/ has if you
>> > really need more than that.
>> >
>> > Don't mess with kernel device names, we can't do that without breaking
>> > tons of things, there's a reason that we standardized on something,
>> > please use it.
>> >
>>
>> Okay, will follow the same. :)
>>
>> One last thing,
>> Why init_name will go away some day ?
>
> Because you should never have a static struct device in the kernel.
> It's a reference counted object that needs to always be dynamically
> allocated in order to properly work. Unfortunatly, there are some users
> who have it allocated statically (on the stack) instead of dynamically
> (on the heap). I'll get rid of them someday, it's _way_ down my list of
> things to do...
>
> thanks,
>
> greg k-h
Thank you so much for your support and answering my queries. :)
Now, the thread has also been marked as solved.
Thanks,
Aniroop
^ permalink raw reply
* device symlinks being removed
From: Patrick Hemmer @ 2014-08-15 23:46 UTC (permalink / raw)
To: linux-hotplug
It seems I have some sort of race condition when using udev rules to set
up some device symlinks, but I'm not sure why.
I'm building EC2 boxes, and immediately after they boot, they add some
udev rules to create symlinks, and then call `udevadm trigger` to
process the rules. The problem is that during this procedure, sometimes
some of the links are missing. The udev debug log indicates the links
were created, and then subsequently removed.
The complete system log can be viewed at http://dpaste.com/1BN5FKA
However I've grabbed what look like the interesting lines, and have
included them at the bottom. Notice the ones such as
creating symlink '/dev/disk/ec2/vol-05b50706' to '../../xvda'
and
update old name, '/dev/disk/ec2/vol-05b50706' no longer belonging to
'/devices/vbd-2048/block/xvda'
no reference left, remove '/dev/disk/ec2/vol-05b50706'
My udev rule looks like this:
SUBSYSTEM="block", SUBSYSTEMS="xen", ATTR{capability}="*",
ACTION="add", PROGRAM="/ec2disks -A %k", SYMLINK+="%c"
My current process is:
# create the file
/bin/udevadm control --reload
/bin/udevadm settle
/bin/udevadm trigger --subsystem-match=block --actiond
The system is using udev 215
The only rule in /etc/udev/rules.d is my rule. All others should be the
standard rules. The distro is CoreOS.
Log:
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: PROGRAM '/ec2disks -A
xvda' /etc/udev/rules.d/10-ec2disks.rules:1
Aug 15 23:13:49 i-863204d8 systemd-udevd[630]: starting '/ec2disks
-A xvda'
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: '/ec2disks -A
xvda'(out) 'disk/ec2/ami disk/ec2/root disk/ec2/vol-05b50706'
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: '/ec2disks -A xvda'
[630] exit with return code 0
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: LINK 'disk/ec2/ami'
/etc/udev/rules.d/10-ec2disks.rules:1
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: LINK 'disk/ec2/root'
/etc/udev/rules.d/10-ec2disks.rules:1
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: LINK
'disk/ec2/vol-05b50706' /etc/udev/rules.d/10-ec2disks.rules:1
Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: IMPORT 'cdrom_id
--lock-media /dev/xvda' /usr/lib64/udev/rules.d/60-cdrom_id.rules:16
Aug 15 23:13:49 i-863204d8 systemd-udevd[642]: starting 'cdrom_id
--lock-media /dev/xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: 'cdrom_id
--lock-media /dev/xvda'(err) 'unable to open '/dev/xvda''
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: 'cdrom_id
--lock-media /dev/xvda' [642] exit with return code 1
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: probe /dev/xvda raid
offset=0
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: handling device node
'/dev/xvda', devnum²02:0, mode\x0660, uid=0, gid=6
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: preserve permissions
/dev/xvda, 060660, uid=0, gid=6
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: preserve already
existing symlink '/dev/block/202:0' to '../xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating link
'/dev/disk/ec2/ami' to '/dev/xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating symlink
'/dev/disk/ec2/ami' to '../../xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating link
'/dev/disk/ec2/root' to '/dev/xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating symlink
'/dev/disk/ec2/root' to '../../xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating link
'/dev/disk/ec2/vol-05b50706' to '/dev/xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating symlink
'/dev/disk/ec2/vol-05b50706' to '../../xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: created db file
'/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: adding watch on
'/dev/xvda'
Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: created db file
'/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: Check if link
configuration needs reloading.
Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: device 0x7f29dc30fc90
has devpath '/devices/vbd-2048/block/xvda'
Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: inotify event: 8 for
/dev/xvda
Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: device 0x7f29dc31f350
has devpath '/devices/vbd-2048/block/xvda'
Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: device /dev/xvda
closed, synthesising 'change'
Aug 15 23:14:31 i-863204d8 systemd-udevd[731]: IMPORT 'cdrom_id
--lock-media /dev/xvda' /usr/lib64/udev/rules.d/60-cdrom_id.rules:16
Aug 15 23:14:31 i-863204d8 systemd-udevd[732]: starting 'cdrom_id
--lock-media /dev/xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: 'cdrom_id
--lock-media /dev/xvda'(err) 'unable to open '/dev/xvda''
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: 'cdrom_id
--lock-media /dev/xvda' [732] exit with return code 1
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: IMPORT builtin
'path_id' /usr/lib64/udev/rules.d/60-persistent-storage.rules:55
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: device 0x7f29dc308070
has devpath '/devices/vbd-2048'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: IMPORT builtin
'path_id' returned non-zero
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: IMPORT builtin
'blkid' /usr/lib64/udev/rules.d/60-persistent-storage.rules:70
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: probe /dev/xvda raid
offset=0
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no db file to read
/run/udev/data/+xen:vbd-2048: No such file or directory
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: update old name,
'/dev/disk/ec2/ami' no longer belonging to '/devices/vbd-2048/block/xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no reference left,
remove '/dev/disk/ec2/ami'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: update old name,
'/dev/disk/ec2/root' no longer belonging to '/devices/vbd-2048/block/xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no reference left,
remove '/dev/disk/ec2/root'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: update old name,
'/dev/disk/ec2/vol-05b50706' no longer belonging to
'/devices/vbd-2048/block/xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no reference left,
remove '/dev/disk/ec2/vol-05b50706'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: handling device node
'/dev/xvda', devnum²02:0, mode\x0600, uid=0, gid=0
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: preserve already
existing symlink '/dev/block/202:0' to '../xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: created db file
'/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: adding watch on
'/dev/xvda'
Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: created db file
'/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
If someone can explain to me why the links are getting removed, I would
appreciate it.
Thanks
-Patrick
^ permalink raw reply
* Re: device symlinks being removed
From: Patrick Hemmer @ 2014-08-16 3:07 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <53EE9BCD.5090101@stormcloud9.net>
*From:* Patrick Hemmer <linux@stormcloud9.net>
*Sent:* 2014-08-15 19:46:21 EDT
*To:* linux-hotplug@vger.kernel.org
*Subject:* device symlinks being removed
> It seems I have some sort of race condition when using udev rules to set
> up some device symlinks, but I'm not sure why.
> I'm building EC2 boxes, and immediately after they boot, they add some
> udev rules to create symlinks, and then call `udevadm trigger` to
> process the rules. The problem is that during this procedure, sometimes
> some of the links are missing. The udev debug log indicates the links
> were created, and then subsequently removed.
>
> The complete system log can be viewed at http://dpaste.com/1BN5FKA
> However I've grabbed what look like the interesting lines, and have
> included them at the bottom. Notice the ones such as
>
> creating symlink '/dev/disk/ec2/vol-05b50706' to '../../xvda'
>
> and
>
> update old name, '/dev/disk/ec2/vol-05b50706' no longer belonging to
> '/devices/vbd-2048/block/xvda'
> no reference left, remove '/dev/disk/ec2/vol-05b50706'
>
>
> My udev rule looks like this:
>
> SUBSYSTEM="block", SUBSYSTEMS="xen", ATTR{capability}="*",
> ACTION="add", PROGRAM="/ec2disks -A %k", SYMLINK+="%c"
>
> My current process is:
>
> # create the file
> /bin/udevadm control --reload
> /bin/udevadm settle
> /bin/udevadm trigger --subsystem-match=block --actiond
>
>
> The system is using udev 215
> The only rule in /etc/udev/rules.d is my rule. All others should be the
> standard rules. The distro is CoreOS.
>
> Log:
>
>
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: PROGRAM '/ec2disks -A
> xvda' /etc/udev/rules.d/10-ec2disks.rules:1
> Aug 15 23:13:49 i-863204d8 systemd-udevd[630]: starting '/ec2disks
> -A xvda'
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: '/ec2disks -A
> xvda'(out) 'disk/ec2/ami disk/ec2/root disk/ec2/vol-05b50706'
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: '/ec2disks -A xvda'
> [630] exit with return code 0
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: LINK 'disk/ec2/ami'
> /etc/udev/rules.d/10-ec2disks.rules:1
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: LINK 'disk/ec2/root'
> /etc/udev/rules.d/10-ec2disks.rules:1
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: LINK
> 'disk/ec2/vol-05b50706' /etc/udev/rules.d/10-ec2disks.rules:1
> Aug 15 23:13:49 i-863204d8 systemd-udevd[629]: IMPORT 'cdrom_id
> --lock-media /dev/xvda' /usr/lib64/udev/rules.d/60-cdrom_id.rules:16
> Aug 15 23:13:49 i-863204d8 systemd-udevd[642]: starting 'cdrom_id
> --lock-media /dev/xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: 'cdrom_id
> --lock-media /dev/xvda'(err) 'unable to open '/dev/xvda''
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: 'cdrom_id
> --lock-media /dev/xvda' [642] exit with return code 1
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: probe /dev/xvda raid
> offset=0
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: handling device node
> '/dev/xvda', devnum²02:0, mode\x0660, uid=0, gid=6
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: preserve permissions
> /dev/xvda, 060660, uid=0, gid=6
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: preserve already
> existing symlink '/dev/block/202:0' to '../xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating link
> '/dev/disk/ec2/ami' to '/dev/xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating symlink
> '/dev/disk/ec2/ami' to '../../xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating link
> '/dev/disk/ec2/root' to '/dev/xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating symlink
> '/dev/disk/ec2/root' to '../../xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating link
> '/dev/disk/ec2/vol-05b50706' to '/dev/xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: creating symlink
> '/dev/disk/ec2/vol-05b50706' to '../../xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: created db file
> '/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: adding watch on
> '/dev/xvda'
> Aug 15 23:13:52 i-863204d8 systemd-udevd[629]: created db file
> '/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: Check if link
> configuration needs reloading.
> Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: device 0x7f29dc30fc90
> has devpath '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: inotify event: 8 for
> /dev/xvda
> Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: device 0x7f29dc31f350
> has devpath '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:31 i-863204d8 systemd-udevd[410]: device /dev/xvda
> closed, synthesising 'change'
> Aug 15 23:14:31 i-863204d8 systemd-udevd[731]: IMPORT 'cdrom_id
> --lock-media /dev/xvda' /usr/lib64/udev/rules.d/60-cdrom_id.rules:16
> Aug 15 23:14:31 i-863204d8 systemd-udevd[732]: starting 'cdrom_id
> --lock-media /dev/xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: 'cdrom_id
> --lock-media /dev/xvda'(err) 'unable to open '/dev/xvda''
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: 'cdrom_id
> --lock-media /dev/xvda' [732] exit with return code 1
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: IMPORT builtin
> 'path_id' /usr/lib64/udev/rules.d/60-persistent-storage.rules:55
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: device 0x7f29dc308070
> has devpath '/devices/vbd-2048'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: IMPORT builtin
> 'path_id' returned non-zero
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: IMPORT builtin
> 'blkid' /usr/lib64/udev/rules.d/60-persistent-storage.rules:70
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: probe /dev/xvda raid
> offset=0
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no db file to read
> /run/udev/data/+xen:vbd-2048: No such file or directory
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: update old name,
> '/dev/disk/ec2/ami' no longer belonging to '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no reference left,
> remove '/dev/disk/ec2/ami'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: update old name,
> '/dev/disk/ec2/root' no longer belonging to '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no reference left,
> remove '/dev/disk/ec2/root'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: update old name,
> '/dev/disk/ec2/vol-05b50706' no longer belonging to
> '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: no reference left,
> remove '/dev/disk/ec2/vol-05b50706'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: handling device node
> '/dev/xvda', devnum²02:0, mode\x0600, uid=0, gid=0
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: preserve already
> existing symlink '/dev/block/202:0' to '../xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: created db file
> '/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: adding watch on
> '/dev/xvda'
> Aug 15 23:14:34 i-863204d8 systemd-udevd[731]: created db file
> '/run/udev/data/b202:0' for '/devices/vbd-2048/block/xvda'
>
>
>
> If someone can explain to me why the links are getting removed, I would
> appreciate it.
>
> Thanks
>
>
> -Patrick
Finally figured out it. For some reason, another event was sometimes
being triggered on the device. My udev rule was matching only 'add'
events, and this other event was apparently something else. So my rule
wouldn't match, and the device would lose the symlinks.
Just removed the ACTION="add" from the udev rule and all appears well.
-Patrick
^ permalink raw reply
* Re: device symlinks being removed
From: Zbigniew Jędrzejewski-Szmek @ 2014-08-18 14:42 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <53EE9BCD.5090101@stormcloud9.net>
On Fri, Aug 15, 2014 at 11:07:05PM -0400, Patrick Hemmer wrote:
> > My udev rule looks like this:
> >
> > SUBSYSTEM="block", SUBSYSTEMS="xen", ATTR{capability}="*",
> > ACTION="add", PROGRAM="/ec2disks -A %k", SYMLINK+="%c"
> Finally figured out it. For some reason, another event was sometimes
> being triggered on the device. My udev rule was matching only 'add'
> events, and this other event was apparently something else. So my rule
> wouldn't match, and the device would lose the symlinks.
>
> Just removed the ACTION="add" from the udev rule and all appears well.
Yep. For a similar case see http://cgit.freedesktop.org/systemd/systemd/commit/?idàd856dd48.
Zbyszek
^ permalink raw reply
* Re: [RFC Patch V1 00/30] Enable memoryless node on x86 platforms
From: Nishanth Aravamudan @ 2014-08-18 23:30 UTC (permalink / raw)
To: Jiang Liu
Cc: Tony Luck, Andrew Morton, Mel Gorman, David Rientjes,
Mike Galbraith, Peter Zijlstra, Rafael J . Wysocki,
linux-mm@kvack.org, linux-hotplug, Linux Kernel Mailing List
In-Reply-To: <53D1B7C9.9040907@linux.intel.com>
Hi Gerry,
On 25.07.2014 [09:50:01 +0800], Jiang Liu wrote:
>
>
> On 2014/7/25 7:32, Nishanth Aravamudan wrote:
> > On 23.07.2014 [16:20:24 +0800], Jiang Liu wrote:
> >>
> >>
> >> On 2014/7/22 1:57, Nishanth Aravamudan wrote:
> >>> On 21.07.2014 [10:41:59 -0700], Tony Luck wrote:
> >>>> On Mon, Jul 21, 2014 at 10:23 AM, Nishanth Aravamudan
> >>>> <nacc@linux.vnet.ibm.com> wrote:
> >>>>> It seems like the issue is the order of onlining of resources on a
> >>>>> specific x86 platform?
> >>>>
> >>>> Yes. When we online a node the BIOS hits us with some ACPI hotplug events:
> >>>>
> >>>> First: Here are some new cpus
> >>>
> >>> Ok, so during this period, you might get some remote allocations. Do you
> >>> know the topology of these CPUs? That is they belong to a
> >>> (soon-to-exist) NUMA node? Can you online that currently offline NUMA
> >>> node at this point (so that NODE_DATA()) resolves, etc.)?
> >> Hi Nishanth,
> >> We have method to get the NUMA information about the CPU, and
> >> patch "[RFC Patch V1 30/30] x86, NUMA: Online node earlier when doing
> >> CPU hot-addition" tries to solve this issue by onlining NUMA node
> >> as early as possible. Actually we are trying to enable memoryless node
> >> as you have suggested.
> >
> > Ok, it seems like you have two sets of patches then? One is to fix the
> > NUMA information timing (30/30 only). The rest of the patches are
> > general discussions about where cpu_to_mem() might be used instead of
> > cpu_to_node(). However, based upon Tejun's feedback, it seems like
> > rather than force all callers to use cpu_to_mem(), we should be looking
> > at the core VM to ensure fallback is occuring appropriately when
> > memoryless nodes are present.
> >
> > Do you have a specific situation, once you've applied 30/30, where
> > kmalloc_node() leads to an Oops?
> Hi Nishanth,
> After following the two threads related to support of memoryless
> node and digging more code, I realized my first version path set is an
> overkill. As Tejun has pointed out, we shouldn't expose the detail of
> memoryless node to normal user, but there are still some special users
> who need the detail. So I have tried to summarize it as:
> 1) Arch code should online corresponding NUMA node before onlining any
> CPU or memory, otherwise it may cause invalid memory access when
> accessing NODE_DATA(nid).
I think that's reasonable.
A related caveat is that NUMA topology information should be stored as
early as possible in boot for *all* CPUs [I think only cpu_to_* is used,
at least for now], not just the boot CPU, etc. This is because (at least
on my examination) pre-SMP initcalls are not prevented from using
cpu_to_node, which will falsely return 0 for all CPUs until
set_cpu_numa_node() is called.
> 2) For normal memory allocations without __GFP_THISNODE setting in the
> gfp_flags, we should prefer numa_node_id()/cpu_to_node() instead of
> numa_mem_id()/cpu_to_mem() because the latter loses hardware topology
> information as pointed out by Tejun:
> A - B - X - C - D
> Where X is the memless node. numa_mem_id() on X would return
> either B or C, right? If B or C can't satisfy the allocation,
> the allocator would fallback to A from B and D for C, both of
> which aren't optimal. It should first fall back to C or B
> respectively, which the allocator can't do anymoe because the
> information is lost when the caller side performs numa_mem_id().
Yes, this seems like a very good description of the reasoning.
> 3) For memory allocation with __GFP_THISNODE setting in gfp_flags,
> numa_node_id()/cpu_to_node() should be used if caller only wants to
> allocate from local memory, otherwise numa_mem_id()/cpu_to_mem()
> should be used if caller wants to allocate from the nearest node.
>
> 4) numa_mem_id()/cpu_to_mem() should be used if caller wants to check
> whether a page is allocated from the nearest node.
I'm less clear on what you mean here, I'll look at your v2 patches. I
mean, numa_node_id()/cpu_to_node() should be used to indicate node-local
preference with appropriate failure handling. But I don't know why one
would prefer to use numa_node_id() to numa_mem_id() in such a path? The
only time they differ is if memoryless nodes are present, which is what
your local memory allocation would ideally be for those nodes anyways?
Thanks,
Nish
^ permalink raw reply
* Improper Naming in /dev/disk/by-id and Drives Offline
From: Brandon R Schwartz @ 2014-09-11 2:34 UTC (permalink / raw)
To: linux-hotplug
Hi,
I'm working on a particular issue (possibly two separate issues) where
our HDDs are (1) getting mislabeled in /dev/disk/by-id and (2)
dropping offline even though drive and controller logs show that the
drive is communicating and working as expected. I don't have much
knowledge on the udev side of things so it would be great if someone
could offer some insight into the way udev assigns device names and if
there are thoughts as to why the OS cannot see the drive in certain
cases (timing issue?).
The first issue, the mislabeling problem, is that on reboots or power
cycles we occasionally see our drives become mislabeled in
/dev/disk/by-id. We expect to see something like:
ata-ST3000DM001-1CH166_W1F26HKK
ata-ST3000DM001-1CH166_Z1F2FBBY
But instead we see:
ata-ST3000DM001-1CH166_W1F26HKK
scsi-35000c500668a9bdb
The "scsi" drive is assigned a drive letter and the OS can communicate
with the drive. Drives logs and controller logs show the drive is
working properly, but for some reason it's getting labeled incorrectly
in /dev/disk/by-id. We have looked through dmesg and enabled logging
in udev (udevadm control --log-priorityfibug), but we have not seen
where these labels are coming from.
The second issue is slightly related to the first in that it appears
during the same power cycle/reboot test. We have noticed that on
occasion, our drives will not be detected by the OS (not listed in
/dev/disk/by-id) at all. However, if we look at drive logs and
controller logs, we don't see any issue. The controller is able to
see the drives and communicate with them, but the OS is unable to.
Any ideas as to why communication is not established?
Also, is there a way to refresh the /dev/disk/by-id listing (udevadm
trigger?) once the OS has booted in order to rescan for attached
devices and repopulate it? Thanks for any information and let me know
if you need logs or anything else.
Regards,
Brandon
--
Brandon Schwartz
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Improper Naming in /dev/disk/by-id and Drives Offline
From: Greg KH @ 2014-09-11 2:53 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <CAFg=k-gDSYG8L68hVky1-j_1xExhzMcX6gobf0a35OtRhdbKVg@mail.gmail.com>
On Wed, Sep 10, 2014 at 08:34:06PM -0600, Brandon R Schwartz wrote:
> Hi,
>
> I'm working on a particular issue (possibly two separate issues) where
> our HDDs are (1) getting mislabeled in /dev/disk/by-id and (2)
> dropping offline even though drive and controller logs show that the
> drive is communicating and working as expected. I don't have much
> knowledge on the udev side of things so it would be great if someone
> could offer some insight into the way udev assigns device names and if
> there are thoughts as to why the OS cannot see the drive in certain
> cases (timing issue?).
>
> The first issue, the mislabeling problem, is that on reboots or power
> cycles we occasionally see our drives become mislabeled in
> /dev/disk/by-id. We expect to see something like:
>
> ata-ST3000DM001-1CH166_W1F26HKK
> ata-ST3000DM001-1CH166_Z1F2FBBY
>
> But instead we see:
>
> ata-ST3000DM001-1CH166_W1F26HKK
> scsi-35000c500668a9bdb
>
> The "scsi" drive is assigned a drive letter and the OS can communicate
> with the drive. Drives logs and controller logs show the drive is
> working properly, but for some reason it's getting labeled incorrectly
> in /dev/disk/by-id. We have looked through dmesg and enabled logging
> in udev (udevadm control --log-priorityÞbug), but we have not seen
> where these labels are coming from.
Sounds like blkid didn't read the uuid properly. Is this happening in
your initrd? Is this a systemd init system, or something else? What
distro / version is this? What kernel version is this?
> The second issue is slightly related to the first in that it appears
> during the same power cycle/reboot test. We have noticed that on
> occasion, our drives will not be detected by the OS (not listed in
> /dev/disk/by-id) at all. However, if we look at drive logs and
> controller logs, we don't see any issue. The controller is able to
> see the drives and communicate with them, but the OS is unable to.
> Any ideas as to why communication is not established?
>
> Also, is there a way to refresh the /dev/disk/by-id listing (udevadm
> trigger?) once the OS has booted in order to rescan for attached
> devices and repopulate it? Thanks for any information and let me know
> if you need logs or anything else.
That depends on your distro, and how it's set up. You could "coldplug"
the by-id values by using udevadmn trigger, have you tried that? You
shouldn't have to do it, as it sounds like you have a boot time race
condition somewhere...
thanks,
greg k-h
^ permalink raw reply
* [Patch Part3 V5 0/8] Enable support of Intel DMAR device hotplug
From: Jiang Liu @ 2014-09-12 2:10 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse, Yinghai Lu, Bjorn Helgaas,
Dan Williams, Vinod Koul, Rafael J . Wysocki
Cc: Jiang Liu, Ashok Raj, Yijing Wang, Tony Luck, iommu, linux-pci,
linux-hotplug, linux-kernel, dmaengine
When hot plugging a descrete IOH or a physical processor with embedded
IIO, we need to handle DMAR(or IOMMU) unit in the PCIe host bridge if
DMAR is in use. This patch set tries to enhance current DMAR/IOMMU/IR
drivers to support hotplug and is based on latest Linus master branch.
All prerequisite patches to support DMAR device hotplug have been merged
into the mainstream kernel, and this is the last patch set to enable
DMAR device hotplug.
You may access the patch set at:
https://github.com/jiangliu/linux.git iommu/hotplug_v5
This patch set has been tested on Intel development machine.
Appreciate any comments and tests.
Patch 1-4 enhances DMAR framework to support hotplug
Patch 5 enhances Intel interrupt remapping driver to support hotplug
Patch 6 enhances error handling in Intel IR driver
Patch 7 enhance Intel IOMMU to support hotplug
Patch 8 enhance ACPI pci_root driver to handle DMAR units
Jiang Liu (8):
iommu/vt-d: Introduce helper function dmar_walk_resources()
iommu/vt-d: Dynamically allocate and free seq_id for DMAR units
iommu/vt-d: Implement DMAR unit hotplug framework
iommu/vt-d: Search for ACPI _DSM method for DMAR hotplug
iommu/vt-d: Enhance intel_irq_remapping driver to support DMAR unit
hotplug
iommu/vt-d: Enhance error recovery in function
intel_enable_irq_remapping()
iommu/vt-d: Enhance intel-iommu driver to support DMAR unit hotplug
pci, ACPI, iommu: Enhance pci_root to support DMAR device hotplug
drivers/acpi/pci_root.c | 16 +-
drivers/iommu/dmar.c | 532 ++++++++++++++++++++++++++++-------
drivers/iommu/intel-iommu.c | 297 ++++++++++++++-----
drivers/iommu/intel_irq_remapping.c | 233 +++++++++++----
include/linux/dmar.h | 50 +++-
5 files changed, 888 insertions(+), 240 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [Patch Part3 V5 1/8] iommu/vt-d: Introduce helper function dmar_walk_resources()
From: Jiang Liu @ 2014-09-12 2:10 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse, Yinghai Lu, Bjorn Helgaas,
Dan Williams, Vinod Koul, Rafael J . Wysocki
Cc: Jiang Liu, Ashok Raj, Yijing Wang, Tony Luck, iommu, linux-pci,
linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <1410487848-6027-1-git-send-email-jiang.liu@linux.intel.com>
Introduce helper function dmar_walk_resources to walk resource entries
in DMAR table and ACPI buffer object returned by ACPI _DSM method
for IOMMU hot-plug.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
drivers/iommu/dmar.c | 209 +++++++++++++++++++++++--------------------
drivers/iommu/intel-iommu.c | 4 +-
include/linux/dmar.h | 19 ++--
3 files changed, 122 insertions(+), 110 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 60ab474bfff3..afd46eb9a5de 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -44,6 +44,14 @@
#include "irq_remapping.h"
+typedef int (*dmar_res_handler_t)(struct acpi_dmar_header *, void *);
+struct dmar_res_callback {
+ dmar_res_handler_t cb[ACPI_DMAR_TYPE_RESERVED];
+ void *arg[ACPI_DMAR_TYPE_RESERVED];
+ bool ignore_unhandled;
+ bool print_entry;
+};
+
/*
* Assumptions:
* 1) The hotplug framework guarentees that DMAR unit will be hot-added
@@ -333,7 +341,7 @@ static struct notifier_block dmar_pci_bus_nb = {
* present in the platform
*/
static int __init
-dmar_parse_one_drhd(struct acpi_dmar_header *header)
+dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
{
struct acpi_dmar_hardware_unit *drhd;
struct dmar_drhd_unit *dmaru;
@@ -364,6 +372,10 @@ dmar_parse_one_drhd(struct acpi_dmar_header *header)
return ret;
}
dmar_register_drhd_unit(dmaru);
+
+ if (arg)
+ (*(int *)arg)++;
+
return 0;
}
@@ -376,7 +388,8 @@ static void dmar_free_drhd(struct dmar_drhd_unit *dmaru)
kfree(dmaru);
}
-static int __init dmar_parse_one_andd(struct acpi_dmar_header *header)
+static int __init dmar_parse_one_andd(struct acpi_dmar_header *header,
+ void *arg)
{
struct acpi_dmar_andd *andd = (void *)header;
@@ -398,7 +411,7 @@ static int __init dmar_parse_one_andd(struct acpi_dmar_header *header)
#ifdef CONFIG_ACPI_NUMA
static int __init
-dmar_parse_one_rhsa(struct acpi_dmar_header *header)
+dmar_parse_one_rhsa(struct acpi_dmar_header *header, void *arg)
{
struct acpi_dmar_rhsa *rhsa;
struct dmar_drhd_unit *drhd;
@@ -425,6 +438,8 @@ dmar_parse_one_rhsa(struct acpi_dmar_header *header)
return 0;
}
+#else
+#define dmar_parse_one_rhsa dmar_res_noop
#endif
static void __init
@@ -486,6 +501,52 @@ static int __init dmar_table_detect(void)
return (ACPI_SUCCESS(status) ? 1 : 0);
}
+static int dmar_walk_resources(struct acpi_dmar_header *start, size_t len,
+ struct dmar_res_callback *cb)
+{
+ int ret = 0;
+ struct acpi_dmar_header *iter, *next;
+ struct acpi_dmar_header *end = ((void *)start) + len;
+
+ for (iter = start; iter < end && ret = 0; iter = next) {
+ next = (void *)iter + iter->length;
+ if (iter->length = 0) {
+ /* Avoid looping forever on bad ACPI tables */
+ pr_debug(FW_BUG "Invalid 0-length structure\n");
+ break;
+ } else if (next > end) {
+ /* Avoid passing table end */
+ pr_warn(FW_BUG "record passes table end\n");
+ ret = -EINVAL;
+ break;
+ }
+
+ if (cb->print_entry)
+ dmar_table_print_dmar_entry(iter);
+
+ if (iter->type >= ACPI_DMAR_TYPE_RESERVED) {
+ /* continue for forward compatibility */
+ pr_debug("Unknown DMAR structure type %d\n",
+ iter->type);
+ } else if (cb->cb[iter->type]) {
+ ret = cb->cb[iter->type](iter, cb->arg[iter->type]);
+ } else if (!cb->ignore_unhandled) {
+ pr_warn("No handler for DMAR structure type %d\n",
+ iter->type);
+ ret = -EINVAL;
+ }
+ }
+
+ return ret;
+}
+
+static inline int dmar_walk_dmar_table(struct acpi_table_dmar *dmar,
+ struct dmar_res_callback *cb)
+{
+ return dmar_walk_resources((struct acpi_dmar_header *)(dmar + 1),
+ dmar->header.length - sizeof(*dmar), cb);
+}
+
/**
* parse_dmar_table - parses the DMA reporting table
*/
@@ -493,9 +554,18 @@ static int __init
parse_dmar_table(void)
{
struct acpi_table_dmar *dmar;
- struct acpi_dmar_header *entry_header;
int ret = 0;
int drhd_count = 0;
+ struct dmar_res_callback cb = {
+ .print_entry = true,
+ .ignore_unhandled = true,
+ .arg[ACPI_DMAR_TYPE_HARDWARE_UNIT] = &drhd_count,
+ .cb[ACPI_DMAR_TYPE_HARDWARE_UNIT] = &dmar_parse_one_drhd,
+ .cb[ACPI_DMAR_TYPE_RESERVED_MEMORY] = &dmar_parse_one_rmrr,
+ .cb[ACPI_DMAR_TYPE_ROOT_ATS] = &dmar_parse_one_atsr,
+ .cb[ACPI_DMAR_TYPE_HARDWARE_AFFINITY] = &dmar_parse_one_rhsa,
+ .cb[ACPI_DMAR_TYPE_NAMESPACE] = &dmar_parse_one_andd,
+ };
/*
* Do it again, earlier dmar_tbl mapping could be mapped with
@@ -519,51 +589,10 @@ parse_dmar_table(void)
}
pr_info("Host address width %d\n", dmar->width + 1);
-
- entry_header = (struct acpi_dmar_header *)(dmar + 1);
- while (((unsigned long)entry_header) <
- (((unsigned long)dmar) + dmar_tbl->length)) {
- /* Avoid looping forever on bad ACPI tables */
- if (entry_header->length = 0) {
- pr_warn("Invalid 0-length structure\n");
- ret = -EINVAL;
- break;
- }
-
- dmar_table_print_dmar_entry(entry_header);
-
- switch (entry_header->type) {
- case ACPI_DMAR_TYPE_HARDWARE_UNIT:
- drhd_count++;
- ret = dmar_parse_one_drhd(entry_header);
- break;
- case ACPI_DMAR_TYPE_RESERVED_MEMORY:
- ret = dmar_parse_one_rmrr(entry_header);
- break;
- case ACPI_DMAR_TYPE_ROOT_ATS:
- ret = dmar_parse_one_atsr(entry_header);
- break;
- case ACPI_DMAR_TYPE_HARDWARE_AFFINITY:
-#ifdef CONFIG_ACPI_NUMA
- ret = dmar_parse_one_rhsa(entry_header);
-#endif
- break;
- case ACPI_DMAR_TYPE_NAMESPACE:
- ret = dmar_parse_one_andd(entry_header);
- break;
- default:
- pr_warn("Unknown DMAR structure type %d\n",
- entry_header->type);
- ret = 0; /* for forward compatibility */
- break;
- }
- if (ret)
- break;
-
- entry_header = ((void *)entry_header + entry_header->length);
- }
- if (drhd_count = 0)
+ ret = dmar_walk_dmar_table(dmar, &cb);
+ if (ret = 0 && drhd_count = 0)
pr_warn(FW_BUG "No DRHD structure found in DMAR table\n");
+
return ret;
}
@@ -762,76 +791,60 @@ static void warn_invalid_dmar(u64 addr, const char *message)
dmi_get_system_info(DMI_PRODUCT_VERSION));
}
-static int __init check_zero_address(void)
+static int __ref
+dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
{
- struct acpi_table_dmar *dmar;
- struct acpi_dmar_header *entry_header;
struct acpi_dmar_hardware_unit *drhd;
+ void __iomem *addr;
+ u64 cap, ecap;
- dmar = (struct acpi_table_dmar *)dmar_tbl;
- entry_header = (struct acpi_dmar_header *)(dmar + 1);
-
- while (((unsigned long)entry_header) <
- (((unsigned long)dmar) + dmar_tbl->length)) {
- /* Avoid looping forever on bad ACPI tables */
- if (entry_header->length = 0) {
- pr_warn("Invalid 0-length structure\n");
- return 0;
- }
-
- if (entry_header->type = ACPI_DMAR_TYPE_HARDWARE_UNIT) {
- void __iomem *addr;
- u64 cap, ecap;
-
- drhd = (void *)entry_header;
- if (!drhd->address) {
- warn_invalid_dmar(0, "");
- goto failed;
- }
+ drhd = (void *)entry;
+ if (!drhd->address) {
+ warn_invalid_dmar(0, "");
+ return -EINVAL;
+ }
- addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
- if (!addr ) {
- printk("IOMMU: can't validate: %llx\n", drhd->address);
- goto failed;
- }
- cap = dmar_readq(addr + DMAR_CAP_REG);
- ecap = dmar_readq(addr + DMAR_ECAP_REG);
- early_iounmap(addr, VTD_PAGE_SIZE);
- if (cap = (uint64_t)-1 && ecap = (uint64_t)-1) {
- warn_invalid_dmar(drhd->address,
- " returns all ones");
- goto failed;
- }
- }
+ addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
+ if (!addr) {
+ pr_warn("IOMMU: can't validate: %llx\n", drhd->address);
+ return -EINVAL;
+ }
+ cap = dmar_readq(addr + DMAR_CAP_REG);
+ ecap = dmar_readq(addr + DMAR_ECAP_REG);
+ early_iounmap(addr, VTD_PAGE_SIZE);
- entry_header = ((void *)entry_header + entry_header->length);
+ if (cap = (uint64_t)-1 && ecap = (uint64_t)-1) {
+ warn_invalid_dmar(drhd->address, " returns all ones");
+ return -EINVAL;
}
- return 1;
-failed:
return 0;
}
int __init detect_intel_iommu(void)
{
int ret;
+ struct dmar_res_callback validate_drhd_cb = {
+ .cb[ACPI_DMAR_TYPE_HARDWARE_UNIT] = &dmar_validate_one_drhd,
+ .ignore_unhandled = true,
+ };
down_write(&dmar_global_lock);
ret = dmar_table_detect();
if (ret)
- ret = check_zero_address();
- {
- if (ret && !no_iommu && !iommu_detected && !dmar_disabled) {
- iommu_detected = 1;
- /* Make sure ACS will be enabled */
- pci_request_acs();
- }
+ ret = !dmar_walk_dmar_table((struct acpi_table_dmar *)dmar_tbl,
+ &validate_drhd_cb);
+ if (ret && !no_iommu && !iommu_detected && !dmar_disabled) {
+ iommu_detected = 1;
+ /* Make sure ACS will be enabled */
+ pci_request_acs();
+ }
#ifdef CONFIG_X86
- if (ret)
- x86_init.iommu.iommu_init = intel_iommu_init;
+ if (ret)
+ x86_init.iommu.iommu_init = intel_iommu_init;
#endif
- }
+
early_acpi_os_unmap_memory((void __iomem *)dmar_tbl, dmar_tbl_size);
dmar_tbl = NULL;
up_write(&dmar_global_lock);
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 5619f264862d..4af2206e41bc 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3682,7 +3682,7 @@ static inline void init_iommu_pm_ops(void) {}
#endif /* CONFIG_PM */
-int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header)
+int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header, void *arg)
{
struct acpi_dmar_reserved_memory *rmrr;
struct dmar_rmrr_unit *rmrru;
@@ -3708,7 +3708,7 @@ int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header)
return 0;
}
-int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr)
+int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr, void *arg)
{
struct acpi_dmar_atsr *atsr;
struct dmar_atsr_unit *atsru;
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 1deece46a0ca..fac8ca34f9a8 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -115,22 +115,21 @@ extern int dmar_remove_dev_scope(struct dmar_pci_notify_info *info,
extern int detect_intel_iommu(void);
extern int enable_drhd_fault_handling(void);
+static inline int dmar_res_noop(struct acpi_dmar_header *hdr, void *arg)
+{
+ return 0;
+}
+
#ifdef CONFIG_INTEL_IOMMU
extern int iommu_detected, no_iommu;
extern int intel_iommu_init(void);
-extern int dmar_parse_one_rmrr(struct acpi_dmar_header *header);
-extern int dmar_parse_one_atsr(struct acpi_dmar_header *header);
+extern int dmar_parse_one_rmrr(struct acpi_dmar_header *header, void *arg);
+extern int dmar_parse_one_atsr(struct acpi_dmar_header *header, void *arg);
extern int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info);
#else /* !CONFIG_INTEL_IOMMU: */
static inline int intel_iommu_init(void) { return -ENODEV; }
-static inline int dmar_parse_one_rmrr(struct acpi_dmar_header *header)
-{
- return 0;
-}
-static inline int dmar_parse_one_atsr(struct acpi_dmar_header *header)
-{
- return 0;
-}
+#define dmar_parse_one_rmrr dmar_res_noop
+#define dmar_parse_one_atsr dmar_res_noop
static inline int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
{
return 0;
--
1.7.10.4
^ permalink raw reply related
* [Patch Part3 V5 2/8] iommu/vt-d: Dynamically allocate and free seq_id for DMAR units
From: Jiang Liu @ 2014-09-12 2:10 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse, Yinghai Lu, Bjorn Helgaas,
Dan Williams, Vinod Koul, Rafael J . Wysocki
Cc: Jiang Liu, Ashok Raj, Yijing Wang, Tony Luck, iommu, linux-pci,
linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <1410487848-6027-1-git-send-email-jiang.liu@linux.intel.com>
Introduce functions to support dynamic IOMMU seq_id allocating and
releasing, which will be used to support DMAR hotplug.
Also rename IOMMU_UNITS_SUPPORTED as DMAR_UNITS_SUPPORTED.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
drivers/iommu/dmar.c | 40 ++++++++++++++++++++++++++++++++++------
drivers/iommu/intel-iommu.c | 13 +++----------
include/linux/dmar.h | 6 ++++++
3 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index afd46eb9a5de..b3405c50627f 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -70,6 +70,7 @@ LIST_HEAD(dmar_drhd_units);
struct acpi_table_header * __initdata dmar_tbl;
static acpi_size dmar_tbl_size;
static int dmar_dev_scope_status = 1;
+static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
static int alloc_iommu(struct dmar_drhd_unit *drhd);
static void free_iommu(struct intel_iommu *iommu);
@@ -928,11 +929,32 @@ out:
return err;
}
+static int dmar_alloc_seq_id(struct intel_iommu *iommu)
+{
+ iommu->seq_id = find_first_zero_bit(dmar_seq_ids,
+ DMAR_UNITS_SUPPORTED);
+ if (iommu->seq_id >= DMAR_UNITS_SUPPORTED) {
+ iommu->seq_id = -1;
+ } else {
+ set_bit(iommu->seq_id, dmar_seq_ids);
+ sprintf(iommu->name, "dmar%d", iommu->seq_id);
+ }
+
+ return iommu->seq_id;
+}
+
+static void dmar_free_seq_id(struct intel_iommu *iommu)
+{
+ if (iommu->seq_id >= 0) {
+ clear_bit(iommu->seq_id, dmar_seq_ids);
+ iommu->seq_id = -1;
+ }
+}
+
static int alloc_iommu(struct dmar_drhd_unit *drhd)
{
struct intel_iommu *iommu;
u32 ver, sts;
- static int iommu_allocated = 0;
int agaw = 0;
int msagaw = 0;
int err;
@@ -946,13 +968,16 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
if (!iommu)
return -ENOMEM;
- iommu->seq_id = iommu_allocated++;
- sprintf (iommu->name, "dmar%d", iommu->seq_id);
+ if (dmar_alloc_seq_id(iommu) < 0) {
+ pr_err("IOMMU: failed to allocate seq_id\n");
+ err = -ENOSPC;
+ goto error;
+ }
err = map_iommu(iommu, drhd->reg_base_addr);
if (err) {
pr_err("IOMMU: failed to map %s\n", iommu->name);
- goto error;
+ goto error_free_seq_id;
}
err = -EINVAL;
@@ -1002,9 +1027,11 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
return 0;
- err_unmap:
+err_unmap:
unmap_iommu(iommu);
- error:
+error_free_seq_id:
+ dmar_free_seq_id(iommu);
+error:
kfree(iommu);
return err;
}
@@ -1028,6 +1055,7 @@ static void free_iommu(struct intel_iommu *iommu)
if (iommu->reg)
unmap_iommu(iommu);
+ dmar_free_seq_id(iommu);
kfree(iommu);
}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4af2206e41bc..7daa74ed46d0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -328,17 +328,10 @@ static int hw_pass_through = 1;
/* si_domain contains mulitple devices */
#define DOMAIN_FLAG_STATIC_IDENTITY (1 << 1)
-/* define the limit of IOMMUs supported in each domain */
-#ifdef CONFIG_X86
-# define IOMMU_UNITS_SUPPORTED MAX_IO_APICS
-#else
-# define IOMMU_UNITS_SUPPORTED 64
-#endif
-
struct dmar_domain {
int id; /* domain id */
int nid; /* node id */
- DECLARE_BITMAP(iommu_bmp, IOMMU_UNITS_SUPPORTED);
+ DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
/* bitmap of iommus this domain uses*/
struct list_head devices; /* all devices' list */
@@ -2728,12 +2721,12 @@ static int __init init_dmars(void)
* threaded kernel __init code path all other access are read
* only
*/
- if (g_num_of_iommus < IOMMU_UNITS_SUPPORTED) {
+ if (g_num_of_iommus < DMAR_UNITS_SUPPORTED) {
g_num_of_iommus++;
continue;
}
printk_once(KERN_ERR "intel-iommu: exceeded %d IOMMUs\n",
- IOMMU_UNITS_SUPPORTED);
+ DMAR_UNITS_SUPPORTED);
}
g_iommus = kcalloc(g_num_of_iommus, sizeof(struct intel_iommu *),
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index fac8ca34f9a8..c8a576bc3a98 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -30,6 +30,12 @@
struct acpi_dmar_header;
+#ifdef CONFIG_X86
+# define DMAR_UNITS_SUPPORTED MAX_IO_APICS
+#else
+# define DMAR_UNITS_SUPPORTED 64
+#endif
+
/* DMAR Flags */
#define DMAR_INTR_REMAP 0x1
#define DMAR_X2APIC_OPT_OUT 0x2
--
1.7.10.4
^ permalink raw reply related
* [Patch Part3 V5 3/8] iommu/vt-d: Implement DMAR unit hotplug framework
From: Jiang Liu @ 2014-09-12 2:10 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse, Yinghai Lu, Bjorn Helgaas,
Dan Williams, Vinod Koul, Rafael J . Wysocki
Cc: Jiang Liu, Ashok Raj, Yijing Wang, Tony Luck, iommu, linux-pci,
linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <1410487848-6027-1-git-send-email-jiang.liu@linux.intel.com>
On Intel platforms, an IO Hub (PCI/PCIe host bridge) may contain DMAR
units, so we need to support DMAR hotplug when supporting PCI host
bridge hotplug on Intel platforms.
According to Section 8.8 "Remapping Hardware Unit Hot Plug" in "Intel
Virtualization Technology for Directed IO Architecture Specification
Rev 2.2", ACPI BIOS should implement ACPI _DSM method under the ACPI
object for the PCI host bridge to support DMAR hotplug.
This patch introduces interfaces to parse ACPI _DSM method for
DMAR unit hotplug. It also implements state machines for DMAR unit
hot-addition and hot-removal.
The PCI host bridge hotplug driver should call dmar_hotplug_hotplug()
before scanning PCI devices connected for hot-addition and after
destroying all PCI devices for hot-removal.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
drivers/iommu/dmar.c | 268 +++++++++++++++++++++++++++++++++--
drivers/iommu/intel-iommu.c | 78 +++++++++-
drivers/iommu/intel_irq_remapping.c | 5 +
include/linux/dmar.h | 33 +++++
4 files changed, 370 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index b3405c50627f..e77b5d3f2f5c 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -75,7 +75,7 @@ static unsigned long dmar_seq_ids[BITS_TO_LONGS(DMAR_UNITS_SUPPORTED)];
static int alloc_iommu(struct dmar_drhd_unit *drhd);
static void free_iommu(struct intel_iommu *iommu);
-static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
+static void dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
{
/*
* add INCLUDE_ALL at the tail, so scan the list will find it at
@@ -336,24 +336,45 @@ static struct notifier_block dmar_pci_bus_nb = {
.priority = INT_MIN,
};
+static struct dmar_drhd_unit *
+dmar_find_dmaru(struct acpi_dmar_hardware_unit *drhd)
+{
+ struct dmar_drhd_unit *dmaru;
+
+ list_for_each_entry_rcu(dmaru, &dmar_drhd_units, list)
+ if (dmaru->segment = drhd->segment &&
+ dmaru->reg_base_addr = drhd->address)
+ return dmaru;
+
+ return NULL;
+}
+
/**
* dmar_parse_one_drhd - parses exactly one DMA remapping hardware definition
* structure which uniquely represent one DMA remapping hardware unit
* present in the platform
*/
-static int __init
-dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
+static int dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
{
struct acpi_dmar_hardware_unit *drhd;
struct dmar_drhd_unit *dmaru;
int ret = 0;
drhd = (struct acpi_dmar_hardware_unit *)header;
- dmaru = kzalloc(sizeof(*dmaru), GFP_KERNEL);
+ dmaru = dmar_find_dmaru(drhd);
+ if (dmaru)
+ goto out;
+
+ dmaru = kzalloc(sizeof(*dmaru) + header->length, GFP_KERNEL);
if (!dmaru)
return -ENOMEM;
- dmaru->hdr = header;
+ /*
+ * If header is allocated from slab by ACPI _DSM method, we need to
+ * copy the content because the memory buffer will be freed on return.
+ */
+ dmaru->hdr = (void *)(dmaru + 1);
+ memcpy(dmaru->hdr, header, header->length);
dmaru->reg_base_addr = drhd->address;
dmaru->segment = drhd->segment;
dmaru->include_all = drhd->flags & 0x1; /* BIT0: INCLUDE_ALL */
@@ -374,6 +395,7 @@ dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
}
dmar_register_drhd_unit(dmaru);
+out:
if (arg)
(*(int *)arg)++;
@@ -411,8 +433,7 @@ static int __init dmar_parse_one_andd(struct acpi_dmar_header *header,
}
#ifdef CONFIG_ACPI_NUMA
-static int __init
-dmar_parse_one_rhsa(struct acpi_dmar_header *header, void *arg)
+static int dmar_parse_one_rhsa(struct acpi_dmar_header *header, void *arg)
{
struct acpi_dmar_rhsa *rhsa;
struct dmar_drhd_unit *drhd;
@@ -805,14 +826,22 @@ dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
return -EINVAL;
}
- addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
+ if (arg)
+ addr = ioremap(drhd->address, VTD_PAGE_SIZE);
+ else
+ addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
if (!addr) {
pr_warn("IOMMU: can't validate: %llx\n", drhd->address);
return -EINVAL;
}
+
cap = dmar_readq(addr + DMAR_CAP_REG);
ecap = dmar_readq(addr + DMAR_ECAP_REG);
- early_iounmap(addr, VTD_PAGE_SIZE);
+
+ if (arg)
+ iounmap(addr);
+ else
+ early_iounmap(addr, VTD_PAGE_SIZE);
if (cap = (uint64_t)-1 && ecap = (uint64_t)-1) {
warn_invalid_dmar(drhd->address, " returns all ones");
@@ -1686,12 +1715,17 @@ int __init dmar_ir_support(void)
return dmar->flags & 0x1;
}
+/* Check whether DMAR units are in use */
+static inline bool dmar_in_use(void)
+{
+ return irq_remapping_enabled || intel_iommu_enabled;
+}
+
static int __init dmar_free_unused_resources(void)
{
struct dmar_drhd_unit *dmaru, *dmaru_n;
- /* DMAR units are in use */
- if (irq_remapping_enabled || intel_iommu_enabled)
+ if (dmar_in_use())
return 0;
if (dmar_dev_scope_status != 1 && !list_empty(&dmar_drhd_units))
@@ -1709,3 +1743,215 @@ static int __init dmar_free_unused_resources(void)
late_initcall(dmar_free_unused_resources);
IOMMU_INIT_POST(detect_intel_iommu);
+
+/*
+ * DMAR Hotplug Support
+ * For more details, please refer to Intel(R) Virtualization Technology
+ * for Directed-IO Architecture Specifiction, Rev 2.2, Section 8.8
+ * "Remapping Hardware Unit Hot Plug".
+ */
+static u8 dmar_hp_uuid[] = {
+ /* 0000 */ 0xA6, 0xA3, 0xC1, 0xD8, 0x9B, 0xBE, 0x9B, 0x4C,
+ /* 0008 */ 0x91, 0xBF, 0xC3, 0xCB, 0x81, 0xFC, 0x5D, 0xAF
+};
+
+/*
+ * Currently there's only one revision and BIOS will not check the revision id,
+ * so use 0 for safety.
+ */
+#define DMAR_DSM_REV_ID 0
+#define DMAR_DSM_FUNC_DRHD 1
+#define DMAR_DSM_FUNC_ATSR 2
+#define DMAR_DSM_FUNC_RHSA 3
+
+static inline bool dmar_detect_dsm(acpi_handle handle, int func)
+{
+ return acpi_check_dsm(handle, dmar_hp_uuid, DMAR_DSM_REV_ID, 1 << func);
+}
+
+static int dmar_walk_dsm_resource(acpi_handle handle, int func,
+ dmar_res_handler_t handler, void *arg)
+{
+ int ret = -ENODEV;
+ union acpi_object *obj;
+ struct acpi_dmar_header *start;
+ struct dmar_res_callback callback;
+ static int res_type[] = {
+ [DMAR_DSM_FUNC_DRHD] = ACPI_DMAR_TYPE_HARDWARE_UNIT,
+ [DMAR_DSM_FUNC_ATSR] = ACPI_DMAR_TYPE_ROOT_ATS,
+ [DMAR_DSM_FUNC_RHSA] = ACPI_DMAR_TYPE_HARDWARE_AFFINITY,
+ };
+
+ if (!dmar_detect_dsm(handle, func))
+ return 0;
+
+ obj = acpi_evaluate_dsm_typed(handle, dmar_hp_uuid, DMAR_DSM_REV_ID,
+ func, NULL, ACPI_TYPE_BUFFER);
+ if (!obj)
+ return -ENODEV;
+
+ memset(&callback, 0, sizeof(callback));
+ callback.cb[res_type[func]] = handler;
+ callback.arg[res_type[func]] = arg;
+ start = (struct acpi_dmar_header *)obj->buffer.pointer;
+ ret = dmar_walk_resources(start, obj->buffer.length, &callback);
+
+ ACPI_FREE(obj);
+
+ return ret;
+}
+
+static int dmar_hp_add_drhd(struct acpi_dmar_header *header, void *arg)
+{
+ int ret;
+ struct dmar_drhd_unit *dmaru;
+
+ dmaru = dmar_find_dmaru((struct acpi_dmar_hardware_unit *)header);
+ if (!dmaru)
+ return -ENODEV;
+
+ ret = dmar_ir_hotplug(dmaru, true);
+ if (ret = 0)
+ ret = dmar_iommu_hotplug(dmaru, true);
+
+ return ret;
+}
+
+static int dmar_hp_remove_drhd(struct acpi_dmar_header *header, void *arg)
+{
+ int i, ret;
+ struct device *dev;
+ struct dmar_drhd_unit *dmaru;
+
+ dmaru = dmar_find_dmaru((struct acpi_dmar_hardware_unit *)header);
+ if (!dmaru)
+ return 0;
+
+ /*
+ * All PCI devices managed by this unit should have been destroyed.
+ */
+ if (!dmaru->include_all && dmaru->devices && dmaru->devices_cnt)
+ for_each_active_dev_scope(dmaru->devices,
+ dmaru->devices_cnt, i, dev)
+ return -EBUSY;
+
+ ret = dmar_ir_hotplug(dmaru, false);
+ if (ret = 0)
+ ret = dmar_iommu_hotplug(dmaru, false);
+
+ return ret;
+}
+
+static int dmar_hp_release_drhd(struct acpi_dmar_header *header, void *arg)
+{
+ struct dmar_drhd_unit *dmaru;
+
+ dmaru = dmar_find_dmaru((struct acpi_dmar_hardware_unit *)header);
+ if (dmaru) {
+ list_del_rcu(&dmaru->list);
+ synchronize_rcu();
+ dmar_free_drhd(dmaru);
+ }
+
+ return 0;
+}
+
+static int dmar_hotplug_insert(acpi_handle handle)
+{
+ int ret;
+ int drhd_count = 0;
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_validate_one_drhd, (void *)1);
+ if (ret)
+ goto out;
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_parse_one_drhd, (void *)&drhd_count);
+ if (ret = 0 && drhd_count = 0) {
+ pr_warn(FW_BUG "No DRHD structures in buffer returned by _DSM method\n");
+ goto out;
+ } else if (ret) {
+ goto release_drhd;
+ }
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_RHSA,
+ &dmar_parse_one_rhsa, NULL);
+ if (ret)
+ goto release_drhd;
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_ATSR,
+ &dmar_parse_one_atsr, NULL);
+ if (ret)
+ goto release_atsr;
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_hp_add_drhd, NULL);
+ if (!ret)
+ return 0;
+
+ dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_hp_remove_drhd, NULL);
+release_atsr:
+ dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_ATSR,
+ &dmar_release_one_atsr, NULL);
+release_drhd:
+ dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_hp_release_drhd, NULL);
+out:
+ return ret;
+}
+
+static int dmar_hotplug_remove(acpi_handle handle)
+{
+ int ret;
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_ATSR,
+ &dmar_check_one_atsr, NULL);
+ if (ret)
+ return ret;
+
+ ret = dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_hp_remove_drhd, NULL);
+ if (ret = 0) {
+ WARN_ON(dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_ATSR,
+ &dmar_release_one_atsr, NULL));
+ WARN_ON(dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_hp_release_drhd, NULL));
+ } else {
+ dmar_walk_dsm_resource(handle, DMAR_DSM_FUNC_DRHD,
+ &dmar_hp_add_drhd, NULL);
+ }
+
+ return ret;
+}
+
+static int dmar_device_hotplug(acpi_handle handle, bool insert)
+{
+ int ret;
+
+ if (!dmar_in_use())
+ return 0;
+
+ if (!dmar_detect_dsm(handle, DMAR_DSM_FUNC_DRHD))
+ return 0;
+
+ down_write(&dmar_global_lock);
+ if (insert)
+ ret = dmar_hotplug_insert(handle);
+ else
+ ret = dmar_hotplug_remove(handle);
+ up_write(&dmar_global_lock);
+
+ return ret;
+}
+
+int dmar_device_add(acpi_handle handle)
+{
+ return dmar_device_hotplug(handle, true);
+}
+
+int dmar_device_remove(acpi_handle handle)
+{
+ return dmar_device_hotplug(handle, false);
+}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 7daa74ed46d0..70d9d47eaeda 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3701,17 +3701,48 @@ int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header, void *arg)
return 0;
}
-int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr, void *arg)
+static struct dmar_atsr_unit *dmar_find_atsr(struct acpi_dmar_atsr *atsr)
+{
+ struct dmar_atsr_unit *atsru;
+ struct acpi_dmar_atsr *tmp;
+
+ list_for_each_entry_rcu(atsru, &dmar_atsr_units, list) {
+ tmp = (struct acpi_dmar_atsr *)atsru->hdr;
+ if (atsr->segment != tmp->segment)
+ continue;
+ if (atsr->header.length != tmp->header.length)
+ continue;
+ if (memcmp(atsr, tmp, atsr->header.length) = 0)
+ return atsru;
+ }
+
+ return NULL;
+}
+
+int dmar_parse_one_atsr(struct acpi_dmar_header *hdr, void *arg)
{
struct acpi_dmar_atsr *atsr;
struct dmar_atsr_unit *atsru;
+ if (system_state != SYSTEM_BOOTING && !intel_iommu_enabled)
+ return 0;
+
atsr = container_of(hdr, struct acpi_dmar_atsr, header);
- atsru = kzalloc(sizeof(*atsru), GFP_KERNEL);
+ atsru = dmar_find_atsr(atsr);
+ if (atsru)
+ return 0;
+
+ atsru = kzalloc(sizeof(*atsru) + hdr->length, GFP_KERNEL);
if (!atsru)
return -ENOMEM;
- atsru->hdr = hdr;
+ /*
+ * If memory is allocated from slab by ACPI _DSM method, we need to
+ * copy the memory content because the memory buffer will be freed
+ * on return.
+ */
+ atsru->hdr = (void *)(atsru + 1);
+ memcpy(atsru->hdr, hdr, hdr->length);
atsru->include_all = atsr->flags & 0x1;
if (!atsru->include_all) {
atsru->devices = dmar_alloc_dev_scope((void *)(atsr + 1),
@@ -3734,6 +3765,47 @@ static void intel_iommu_free_atsr(struct dmar_atsr_unit *atsru)
kfree(atsru);
}
+int dmar_release_one_atsr(struct acpi_dmar_header *hdr, void *arg)
+{
+ struct acpi_dmar_atsr *atsr;
+ struct dmar_atsr_unit *atsru;
+
+ atsr = container_of(hdr, struct acpi_dmar_atsr, header);
+ atsru = dmar_find_atsr(atsr);
+ if (atsru) {
+ list_del_rcu(&atsru->list);
+ synchronize_rcu();
+ intel_iommu_free_atsr(atsru);
+ }
+
+ return 0;
+}
+
+int dmar_check_one_atsr(struct acpi_dmar_header *hdr, void *arg)
+{
+ int i;
+ struct device *dev;
+ struct acpi_dmar_atsr *atsr;
+ struct dmar_atsr_unit *atsru;
+
+ atsr = container_of(hdr, struct acpi_dmar_atsr, header);
+ atsru = dmar_find_atsr(atsr);
+ if (!atsru)
+ return 0;
+
+ if (!atsru->include_all && atsru->devices && atsru->devices_cnt)
+ for_each_active_dev_scope(atsru->devices, atsru->devices_cnt,
+ i, dev)
+ return -EBUSY;
+
+ return 0;
+}
+
+int dmar_iommu_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
+{
+ return intel_iommu_enabled ? -ENOSYS : 0;
+}
+
static void intel_iommu_free_dmars(void)
{
struct dmar_rmrr_unit *rmrru, *rmrr_n;
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 0df41f6264f5..9b140ed854ec 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -1172,3 +1172,8 @@ struct irq_remap_ops intel_irq_remap_ops = {
.msi_setup_irq = intel_msi_setup_irq,
.setup_hpet_msi = intel_setup_hpet_msi,
};
+
+int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
+{
+ return irq_remapping_enabled ? -ENOSYS : 0;
+}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index c8a576bc3a98..594d4ac79e75 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -120,6 +120,8 @@ extern int dmar_remove_dev_scope(struct dmar_pci_notify_info *info,
/* Intel IOMMU detection */
extern int detect_intel_iommu(void);
extern int enable_drhd_fault_handling(void);
+extern int dmar_device_add(acpi_handle handle);
+extern int dmar_device_remove(acpi_handle handle);
static inline int dmar_res_noop(struct acpi_dmar_header *hdr, void *arg)
{
@@ -131,17 +133,48 @@ extern int iommu_detected, no_iommu;
extern int intel_iommu_init(void);
extern int dmar_parse_one_rmrr(struct acpi_dmar_header *header, void *arg);
extern int dmar_parse_one_atsr(struct acpi_dmar_header *header, void *arg);
+extern int dmar_check_one_atsr(struct acpi_dmar_header *hdr, void *arg);
+extern int dmar_release_one_atsr(struct acpi_dmar_header *hdr, void *arg);
+extern int dmar_iommu_hotplug(struct dmar_drhd_unit *dmaru, bool insert);
extern int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info);
#else /* !CONFIG_INTEL_IOMMU: */
static inline int intel_iommu_init(void) { return -ENODEV; }
+
#define dmar_parse_one_rmrr dmar_res_noop
#define dmar_parse_one_atsr dmar_res_noop
+#define dmar_check_one_atsr dmar_res_noop
+#define dmar_release_one_atsr dmar_res_noop
+
static inline int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
{
return 0;
}
+
+static inline int dmar_iommu_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
+{
+ return 0;
+}
#endif /* CONFIG_INTEL_IOMMU */
+#ifdef CONFIG_IRQ_REMAP
+extern int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool insert);
+#else /* CONFIG_IRQ_REMAP */
+static inline int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
+{ return 0; }
+#endif /* CONFIG_IRQ_REMAP */
+
+#else /* CONFIG_DMAR_TABLE */
+
+static inline int dmar_device_add(void *handle)
+{
+ return 0;
+}
+
+static inline int dmar_device_remove(void *handle)
+{
+ return 0;
+}
+
#endif /* CONFIG_DMAR_TABLE */
struct irte {
--
1.7.10.4
^ permalink raw reply related
* [Patch Part3 V5 4/8] iommu/vt-d: Search for ACPI _DSM method for DMAR hotplug
From: Jiang Liu @ 2014-09-12 2:10 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse, Yinghai Lu, Bjorn Helgaas,
Dan Williams, Vinod Koul, Rafael J . Wysocki
Cc: Jiang Liu, Ashok Raj, Yijing Wang, Tony Luck, iommu, linux-pci,
linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <1410487848-6027-1-git-send-email-jiang.liu@linux.intel.com>
According to Intel VT-d specification, _DSM method to support DMAR
hotplug should exist directly under corresponding ACPI object
representing PCI host bridge. But some BIOSes doesn't conform to
this, so search for _DSM method in the subtree starting from the
ACPI object representing the PCI host bridge.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
drivers/iommu/dmar.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index e77b5d3f2f5c..df2c2591c1a6 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1926,21 +1926,48 @@ static int dmar_hotplug_remove(acpi_handle handle)
return ret;
}
-static int dmar_device_hotplug(acpi_handle handle, bool insert)
+static acpi_status dmar_get_dsm_handle(acpi_handle handle, u32 lvl,
+ void *context, void **retval)
+{
+ acpi_handle *phdl = retval;
+
+ if (dmar_detect_dsm(handle, DMAR_DSM_FUNC_DRHD)) {
+ *phdl = handle;
+ return AE_CTRL_TERMINATE;
+ }
+
+ return AE_OK;
+}
+
+int dmar_device_hotplug(acpi_handle handle, bool insert)
{
int ret;
+ acpi_handle tmp = NULL;
+ acpi_status status;
if (!dmar_in_use())
return 0;
- if (!dmar_detect_dsm(handle, DMAR_DSM_FUNC_DRHD))
+ if (dmar_detect_dsm(handle, DMAR_DSM_FUNC_DRHD)) {
+ tmp = handle;
+ } else {
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
+ ACPI_UINT32_MAX,
+ dmar_get_dsm_handle,
+ NULL, NULL, &tmp);
+ if (ACPI_FAILURE(status)) {
+ pr_warn("Failed to locate _DSM method.\n");
+ return -ENXIO;
+ }
+ }
+ if (tmp = NULL)
return 0;
down_write(&dmar_global_lock);
if (insert)
- ret = dmar_hotplug_insert(handle);
+ ret = dmar_hotplug_insert(tmp);
else
- ret = dmar_hotplug_remove(handle);
+ ret = dmar_hotplug_remove(tmp);
up_write(&dmar_global_lock);
return ret;
--
1.7.10.4
^ permalink raw reply related
* [Patch Part3 V5 5/8] iommu/vt-d: Enhance intel_irq_remapping driver to support DMAR unit hotplug
From: Jiang Liu @ 2014-09-12 2:10 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse, Yinghai Lu, Bjorn Helgaas,
Dan Williams, Vinod Koul, Rafael J . Wysocki
Cc: Jiang Liu, Ashok Raj, Yijing Wang, Tony Luck, iommu, linux-pci,
linux-hotplug, linux-kernel, dmaengine
In-Reply-To: <1410487848-6027-1-git-send-email-jiang.liu@linux.intel.com>
Implement required callback functions for intel_irq_remapping driver
to support DMAR unit hotplug.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
drivers/iommu/intel_irq_remapping.c | 222 ++++++++++++++++++++++++++---------
1 file changed, 169 insertions(+), 53 deletions(-)
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 9b140ed854ec..7cf31a29f77a 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -36,7 +36,6 @@ struct hpet_scope {
static struct ioapic_scope ir_ioapic[MAX_IO_APICS];
static struct hpet_scope ir_hpet[MAX_HPET_TBS];
-static int ir_ioapic_num, ir_hpet_num;
/*
* Lock ordering:
@@ -325,7 +324,7 @@ static int set_ioapic_sid(struct irte *irte, int apic)
down_read(&dmar_global_lock);
for (i = 0; i < MAX_IO_APICS; i++) {
- if (ir_ioapic[i].id = apic) {
+ if (ir_ioapic[i].iommu && ir_ioapic[i].id = apic) {
sid = (ir_ioapic[i].bus << 8) | ir_ioapic[i].devfn;
break;
}
@@ -352,7 +351,7 @@ static int set_hpet_sid(struct irte *irte, u8 id)
down_read(&dmar_global_lock);
for (i = 0; i < MAX_HPET_TBS; i++) {
- if (ir_hpet[i].id = id) {
+ if (ir_hpet[i].iommu && ir_hpet[i].id = id) {
sid = (ir_hpet[i].bus << 8) | ir_hpet[i].devfn;
break;
}
@@ -474,17 +473,17 @@ static void iommu_set_irq_remapping(struct intel_iommu *iommu, int mode)
raw_spin_unlock_irqrestore(&iommu->register_lock, flags);
}
-
-static int intel_setup_irq_remapping(struct intel_iommu *iommu, int mode)
+static int intel_setup_irq_remapping(struct intel_iommu *iommu)
{
struct ir_table *ir_table;
struct page *pages;
unsigned long *bitmap;
- ir_table = iommu->ir_table = kzalloc(sizeof(struct ir_table),
- GFP_ATOMIC);
+ if (iommu->ir_table)
+ return 0;
- if (!iommu->ir_table)
+ ir_table = kzalloc(sizeof(struct ir_table), GFP_ATOMIC);
+ if (!ir_table)
return -ENOMEM;
pages = alloc_pages_node(iommu->node, GFP_ATOMIC | __GFP_ZERO,
@@ -493,7 +492,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu, int mode)
if (!pages) {
pr_err("IR%d: failed to allocate pages of order %d\n",
iommu->seq_id, INTR_REMAP_PAGE_ORDER);
- kfree(iommu->ir_table);
+ kfree(ir_table);
return -ENOMEM;
}
@@ -508,11 +507,22 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu, int mode)
ir_table->base = page_address(pages);
ir_table->bitmap = bitmap;
+ iommu->ir_table = ir_table;
- iommu_set_irq_remapping(iommu, mode);
return 0;
}
+static void intel_teardown_irq_remapping(struct intel_iommu *iommu)
+{
+ if (iommu && iommu->ir_table) {
+ free_pages((unsigned long)iommu->ir_table->base,
+ INTR_REMAP_PAGE_ORDER);
+ kfree(iommu->ir_table->bitmap);
+ kfree(iommu->ir_table);
+ iommu->ir_table = NULL;
+ }
+}
+
/*
* Disable Interrupt Remapping.
*/
@@ -667,9 +677,10 @@ static int __init intel_enable_irq_remapping(void)
if (!ecap_ir_support(iommu->ecap))
continue;
- if (intel_setup_irq_remapping(iommu, eim))
+ if (intel_setup_irq_remapping(iommu))
goto error;
+ iommu_set_irq_remapping(iommu, eim);
setup = 1;
}
@@ -700,12 +711,13 @@ error:
return -1;
}
-static void ir_parse_one_hpet_scope(struct acpi_dmar_device_scope *scope,
- struct intel_iommu *iommu)
+static int ir_parse_one_hpet_scope(struct acpi_dmar_device_scope *scope,
+ struct intel_iommu *iommu,
+ struct acpi_dmar_hardware_unit *drhd)
{
struct acpi_dmar_pci_path *path;
u8 bus;
- int count;
+ int count, free = -1;
bus = scope->bus;
path = (struct acpi_dmar_pci_path *)(scope + 1);
@@ -721,19 +733,36 @@ static void ir_parse_one_hpet_scope(struct acpi_dmar_device_scope *scope,
PCI_SECONDARY_BUS);
path++;
}
- ir_hpet[ir_hpet_num].bus = bus;
- ir_hpet[ir_hpet_num].devfn = PCI_DEVFN(path->device, path->function);
- ir_hpet[ir_hpet_num].iommu = iommu;
- ir_hpet[ir_hpet_num].id = scope->enumeration_id;
- ir_hpet_num++;
+
+ for (count = 0; count < MAX_HPET_TBS; count++) {
+ if (ir_hpet[count].iommu = iommu &&
+ ir_hpet[count].id = scope->enumeration_id)
+ return 0;
+ else if (ir_hpet[count].iommu = NULL && free = -1)
+ free = count;
+ }
+ if (free = -1) {
+ pr_warn("Exceeded Max HPET blocks\n");
+ return -ENOSPC;
+ }
+
+ ir_hpet[free].iommu = iommu;
+ ir_hpet[free].id = scope->enumeration_id;
+ ir_hpet[free].bus = bus;
+ ir_hpet[free].devfn = PCI_DEVFN(path->device, path->function);
+ pr_info("HPET id %d under DRHD base 0x%Lx\n",
+ scope->enumeration_id, drhd->address);
+
+ return 0;
}
-static void ir_parse_one_ioapic_scope(struct acpi_dmar_device_scope *scope,
- struct intel_iommu *iommu)
+static int ir_parse_one_ioapic_scope(struct acpi_dmar_device_scope *scope,
+ struct intel_iommu *iommu,
+ struct acpi_dmar_hardware_unit *drhd)
{
struct acpi_dmar_pci_path *path;
u8 bus;
- int count;
+ int count, free = -1;
bus = scope->bus;
path = (struct acpi_dmar_pci_path *)(scope + 1);
@@ -750,54 +779,63 @@ static void ir_parse_one_ioapic_scope(struct acpi_dmar_device_scope *scope,
path++;
}
- ir_ioapic[ir_ioapic_num].bus = bus;
- ir_ioapic[ir_ioapic_num].devfn = PCI_DEVFN(path->device, path->function);
- ir_ioapic[ir_ioapic_num].iommu = iommu;
- ir_ioapic[ir_ioapic_num].id = scope->enumeration_id;
- ir_ioapic_num++;
+ for (count = 0; count < MAX_IO_APICS; count++) {
+ if (ir_ioapic[count].iommu = iommu &&
+ ir_ioapic[count].id = scope->enumeration_id)
+ return 0;
+ else if (ir_ioapic[count].iommu = NULL && free = -1)
+ free = count;
+ }
+ if (free = -1) {
+ pr_warn("Exceeded Max IO APICS\n");
+ return -ENOSPC;
+ }
+
+ ir_ioapic[free].bus = bus;
+ ir_ioapic[free].devfn = PCI_DEVFN(path->device, path->function);
+ ir_ioapic[free].iommu = iommu;
+ ir_ioapic[free].id = scope->enumeration_id;
+ pr_info("IOAPIC id %d under DRHD base 0x%Lx IOMMU %d\n",
+ scope->enumeration_id, drhd->address, iommu->seq_id);
+
+ return 0;
}
static int ir_parse_ioapic_hpet_scope(struct acpi_dmar_header *header,
struct intel_iommu *iommu)
{
+ int ret = 0;
struct acpi_dmar_hardware_unit *drhd;
struct acpi_dmar_device_scope *scope;
void *start, *end;
drhd = (struct acpi_dmar_hardware_unit *)header;
-
start = (void *)(drhd + 1);
end = ((void *)drhd) + header->length;
- while (start < end) {
+ while (start < end && ret = 0) {
scope = start;
- if (scope->entry_type = ACPI_DMAR_SCOPE_TYPE_IOAPIC) {
- if (ir_ioapic_num = MAX_IO_APICS) {
- printk(KERN_WARNING "Exceeded Max IO APICS\n");
- return -1;
- }
-
- printk(KERN_INFO "IOAPIC id %d under DRHD base "
- " 0x%Lx IOMMU %d\n", scope->enumeration_id,
- drhd->address, iommu->seq_id);
+ if (scope->entry_type = ACPI_DMAR_SCOPE_TYPE_IOAPIC)
+ ret = ir_parse_one_ioapic_scope(scope, iommu, drhd);
+ else if (scope->entry_type = ACPI_DMAR_SCOPE_TYPE_HPET)
+ ret = ir_parse_one_hpet_scope(scope, iommu, drhd);
+ start += scope->length;
+ }
- ir_parse_one_ioapic_scope(scope, iommu);
- } else if (scope->entry_type = ACPI_DMAR_SCOPE_TYPE_HPET) {
- if (ir_hpet_num = MAX_HPET_TBS) {
- printk(KERN_WARNING "Exceeded Max HPET blocks\n");
- return -1;
- }
+ return ret;
+}
- printk(KERN_INFO "HPET id %d under DRHD base"
- " 0x%Lx\n", scope->enumeration_id,
- drhd->address);
+static void ir_remove_ioapic_hpet_scope(struct intel_iommu *iommu)
+{
+ int i;
- ir_parse_one_hpet_scope(scope, iommu);
- }
- start += scope->length;
- }
+ for (i = 0; i < MAX_HPET_TBS; i++)
+ if (ir_hpet[i].iommu = iommu)
+ ir_hpet[i].iommu = NULL;
- return 0;
+ for (i = 0; i < MAX_IO_APICS; i++)
+ if (ir_ioapic[i].iommu = iommu)
+ ir_ioapic[i].iommu = NULL;
}
/*
@@ -1173,7 +1211,85 @@ struct irq_remap_ops intel_irq_remap_ops = {
.setup_hpet_msi = intel_setup_hpet_msi,
};
+/*
+ * Support of Interrupt Remapping Unit Hotplug
+ */
+static int dmar_ir_add(struct dmar_drhd_unit *dmaru, struct intel_iommu *iommu)
+{
+ int ret;
+ int eim = x2apic_enabled();
+
+ if (eim && !ecap_eim_support(iommu->ecap)) {
+ pr_info("DRHD %Lx: EIM not supported by DRHD, ecap %Lx\n",
+ iommu->reg_phys, iommu->ecap);
+ return -ENODEV;
+ }
+
+ if (ir_parse_ioapic_hpet_scope(dmaru->hdr, iommu)) {
+ pr_warn("DRHD %Lx: failed to parse managed IOAPIC/HPET\n",
+ iommu->reg_phys);
+ return -ENODEV;
+ }
+
+ /* TODO: check all IOAPICs are covered by IOMMU */
+
+ /* Setup Interrupt-remapping now. */
+ ret = intel_setup_irq_remapping(iommu);
+ if (ret) {
+ pr_err("DRHD %Lx: failed to allocate resource\n",
+ iommu->reg_phys);
+ ir_remove_ioapic_hpet_scope(iommu);
+ return ret;
+ }
+
+ if (!iommu->qi) {
+ /* Clear previous faults. */
+ dmar_fault(-1, iommu);
+ iommu_disable_irq_remapping(iommu);
+ dmar_disable_qi(iommu);
+ }
+
+ /* Enable queued invalidation */
+ ret = dmar_enable_qi(iommu);
+ if (!ret) {
+ iommu_set_irq_remapping(iommu, eim);
+ } else {
+ pr_err("DRHD %Lx: failed to enable queued invalidation, ecap %Lx, ret %d\n",
+ iommu->reg_phys, iommu->ecap, ret);
+ intel_teardown_irq_remapping(iommu);
+ ir_remove_ioapic_hpet_scope(iommu);
+ }
+
+ return ret;
+}
+
int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
{
- return irq_remapping_enabled ? -ENOSYS : 0;
+ int ret = 0;
+ struct intel_iommu *iommu = dmaru->iommu;
+
+ if (!irq_remapping_enabled)
+ return 0;
+ if (iommu = NULL)
+ return -EINVAL;
+ if (!ecap_ir_support(iommu->ecap))
+ return 0;
+
+ if (insert) {
+ if (!iommu->ir_table)
+ ret = dmar_ir_add(dmaru, iommu);
+ } else {
+ if (iommu->ir_table) {
+ if (!bitmap_empty(iommu->ir_table->bitmap,
+ INTR_REMAP_TABLE_ENTRIES)) {
+ ret = -EBUSY;
+ } else {
+ iommu_disable_irq_remapping(iommu);
+ intel_teardown_irq_remapping(iommu);
+ ir_remove_ioapic_hpet_scope(iommu);
+ }
+ }
+ }
+
+ return ret;
}
--
1.7.10.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox