From: Wan ZongShun <mcuos.com@gmail.com> To: Joerg Roedel <joro@8bytes.org> Cc: Wan Zongshun <vincent.wan@amd.com>, linux-kernel <linux-kernel@vger.kernel.org>, iommu@lists.linux-foundation.org, Ray Huang <ray.huang@amd.com>, Borislav Petkov <bp@suse.de>, ken.xue@amd.com, vw@iommu.org Subject: Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices Date: Fri, 8 Jan 2016 11:15:37 +0800 [thread overview] Message-ID: <CAKT61h9retMxx8JiWEHYRVkX7ROXVvsYDJ3WZDoL17YhdjJdBA@mail.gmail.com> (raw) In-Reply-To: <20160107120422.GB19149@8bytes.org> 2016-01-07 20:04 GMT+08:00 Joerg Roedel <joro@8bytes.org>: > On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote: >> -static inline u16 get_device_id(struct device *dev) >> +static inline int match_hid_uid(struct device *dev, >> + struct acpihid_map_entry *entry) >> +{ >> + const char *hid, *uid; >> + >> + hid = acpi_device_hid(ACPI_COMPANION(dev)); >> + uid = acpi_device_uid(ACPI_COMPANION(dev)); >> + >> + if (!hid || !(*hid)) >> + return -ENODEV; >> + >> + if (!uid || !(*uid)) >> + return strcmp(hid, entry->hid); >> + >> + if (!(*entry->uid)) >> + return strcmp(hid, entry->hid); >> + >> + return -ENODEV; >> +} >> + >> +static inline u16 get_pci_device_id(struct device *dev) >> { >> struct pci_dev *pdev = to_pci_dev(dev); >> >> return PCI_DEVID(pdev->bus->number, pdev->devfn); >> } >> >> +static inline int get_acpihid_device_id(struct device *dev, >> + struct acpihid_map_entry **entry) >> +{ >> + struct acpihid_map_entry *p; >> + >> + list_for_each_entry(p, &acpihid_map, list) { >> + if (!match_hid_uid(dev, p)) { >> + if (entry) >> + *entry = p; >> + return p->devid; >> + } >> + } >> + return -EINVAL; >> +} >> + >> +static inline u16 get_device_id(struct device *dev) >> +{ >> + if (dev_is_pci(dev)) >> + return get_pci_device_id(dev); >> + else >> + return get_acpihid_device_id(dev, NULL); >> +} > > This is not robust, get_acpihid_device_id() returns int and can return a > negative value. This gets lost when converting it to u16 here. So either > you add error handling for get_acpihid_device_id() in get_device_id() or > you change get_device_id() to return int too and handle the error at the > callers of get_device_id(). Joerg, Please see the following function, since I judge this 'get_acpihid_device_id(dev, NULL) < 0' in the front of 'get_device_id', so your concern should not exist. I have already filtered the negative situation in check_device firstly, do you think it is ok? static bool check_device(struct device *dev) { u16 devid; ...... /* No PCI device */ if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0)) return false; devid = get_device_id(dev); ..... return true; } > > > Joerg > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu -- --- Vincent Wan(Zongshun) www.mcuos.com
WARNING: multiple messages have this Message-ID (diff)
From: Wan ZongShun <mcuos.com-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> Cc: vw-6ukY98dZOFrYtjvyW6yDsg@public.gmane.org, linux-kernel <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Wan Zongshun <vincent.wan-5C7GfCeVMHo@public.gmane.org>, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Ray Huang <ray.huang-5C7GfCeVMHo@public.gmane.org>, Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>, ken.xue-5C7GfCeVMHo@public.gmane.org Subject: Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices Date: Fri, 8 Jan 2016 11:15:37 +0800 [thread overview] Message-ID: <CAKT61h9retMxx8JiWEHYRVkX7ROXVvsYDJ3WZDoL17YhdjJdBA@mail.gmail.com> (raw) In-Reply-To: <20160107120422.GB19149-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2016-01-07 20:04 GMT+08:00 Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>: > On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote: >> -static inline u16 get_device_id(struct device *dev) >> +static inline int match_hid_uid(struct device *dev, >> + struct acpihid_map_entry *entry) >> +{ >> + const char *hid, *uid; >> + >> + hid = acpi_device_hid(ACPI_COMPANION(dev)); >> + uid = acpi_device_uid(ACPI_COMPANION(dev)); >> + >> + if (!hid || !(*hid)) >> + return -ENODEV; >> + >> + if (!uid || !(*uid)) >> + return strcmp(hid, entry->hid); >> + >> + if (!(*entry->uid)) >> + return strcmp(hid, entry->hid); >> + >> + return -ENODEV; >> +} >> + >> +static inline u16 get_pci_device_id(struct device *dev) >> { >> struct pci_dev *pdev = to_pci_dev(dev); >> >> return PCI_DEVID(pdev->bus->number, pdev->devfn); >> } >> >> +static inline int get_acpihid_device_id(struct device *dev, >> + struct acpihid_map_entry **entry) >> +{ >> + struct acpihid_map_entry *p; >> + >> + list_for_each_entry(p, &acpihid_map, list) { >> + if (!match_hid_uid(dev, p)) { >> + if (entry) >> + *entry = p; >> + return p->devid; >> + } >> + } >> + return -EINVAL; >> +} >> + >> +static inline u16 get_device_id(struct device *dev) >> +{ >> + if (dev_is_pci(dev)) >> + return get_pci_device_id(dev); >> + else >> + return get_acpihid_device_id(dev, NULL); >> +} > > This is not robust, get_acpihid_device_id() returns int and can return a > negative value. This gets lost when converting it to u16 here. So either > you add error handling for get_acpihid_device_id() in get_device_id() or > you change get_device_id() to return int too and handle the error at the > callers of get_device_id(). Joerg, Please see the following function, since I judge this 'get_acpihid_device_id(dev, NULL) < 0' in the front of 'get_device_id', so your concern should not exist. I have already filtered the negative situation in check_device firstly, do you think it is ok? static bool check_device(struct device *dev) { u16 devid; ...... /* No PCI device */ if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0)) return false; devid = get_device_id(dev); ..... return true; } > > > Joerg > > _______________________________________________ > iommu mailing list > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu -- --- Vincent Wan(Zongshun) www.mcuos.com
next prev parent reply other threads:[~2016-01-08 3:15 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-05 10:07 [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-05 10:07 ` [PATCH 1/6] iommu/amd: Modify ivhd_header structure to support type 11h and 40h Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-05 10:07 ` [PATCH 2/6] iommu/amd: Use the most comprehensive IVHD type that the driver can support Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-05 10:07 ` [PATCH 3/6] iommu/amd: Add new map for storing IVHD dev entry type HID Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-05 10:07 ` [PATCH 4/6] iommu/amd: Introduces ivrs_acpihid kernel parameter Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-05 10:07 ` [PATCH 5/6] iommu/amd: Add support for non-pci devices Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-07 12:04 ` Joerg Roedel 2016-01-07 12:04 ` Joerg Roedel 2016-01-08 3:15 ` Wan ZongShun [this message] 2016-01-08 3:15 ` Wan ZongShun 2016-01-08 12:18 ` Joerg Roedel 2016-01-08 14:52 ` Wan Zongshun 2016-01-08 14:52 ` Wan Zongshun 2016-01-08 17:01 ` Joerg Roedel 2016-01-08 17:01 ` Joerg Roedel 2016-01-09 9:47 ` Wan Zongshun 2016-01-20 12:00 ` Joerg Roedel 2016-01-05 10:07 ` [PATCH 6/6] iommu/amd: Manage iommu_group " Wan Zongshun 2016-01-05 10:07 ` Wan Zongshun 2016-01-07 12:06 ` Joerg Roedel 2016-01-07 12:06 ` Joerg Roedel 2016-01-08 1:44 ` Wan ZongShun 2016-01-05 15:40 ` [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support Suravee Suthikulpanit 2016-01-05 15:40 ` Suravee Suthikulpanit
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAKT61h9retMxx8JiWEHYRVkX7ROXVvsYDJ3WZDoL17YhdjJdBA@mail.gmail.com \ --to=mcuos.com@gmail.com \ --cc=bp@suse.de \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=ken.xue@amd.com \ --cc=linux-kernel@vger.kernel.org \ --cc=ray.huang@amd.com \ --cc=vincent.wan@amd.com \ --cc=vw@iommu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.