2023-12-29 03:57:55

by Takashi Sakamoto

[permalink] [raw]
Subject: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards

VIA VT6306/6307/6308 provides PCI interface compliant to 1394 OHCI. When
the hardware is combined with Asmedia ASM1083/1085 PCIe-to-PCI bus bridge,
it appears that accesses to its 'Isochronous Cycle Timer' register (offset
0xf0 on PCI I/O space) often causes unexpected system reboot in any type
of AMD Ryzen machine (both 0x17 and 0x19 families). It does not appears in
the other type of machine (AMD pre-Ryzen machine, Intel machine, at least),
or in the other OHCI 1394 hardware (e.g. Texas Instruments).

The issue explicitly appears at a commit dcadfd7f7c74 ("firewire: core:
use union for callback of transaction completion") added to v6.5 kernel.
It changed 1394 OHCI driver to access to the register every time to
dispatch local asynchronous transaction. However, the issue exists in
older version of kernel as long as it runs in AMD Ryzen machine, since
the access to the register is required to maintain bus time. It is not
hard to imagine that users experience the unexpected system reboot when
generating bus reset by plugging any devices in, or reading the register
by time-aware application programs; e.g. audio sample processing.

Well, this commit suppresses the system reboot in the combination of
hardware. It avoids the access itself. As a result, the software stack can
not provide the hardware time anymore to unit drivers, userspace
applications, and nodes in the same IEEE 1394 bus. It brings apparent
disadvantage since time-aware application programs require it, while
time-unaware applications are available again; e.g. sbp2.

Cc: [email protected]
Reported-by: Jiri Slaby <[email protected]>
Closes: https://bugzilla.suse.com/show_bug.cgi?id=1215436
Reported-by: Mario Limonciello <[email protected]>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217994
Reported-by: Tobias Gruetzmacher <[email protected]>
Closes: https://sourceforge.net/p/linux1394/mailman/message/58711901/
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2240973
Closes: https://bugs.launchpad.net/linux/+bug/2043905
Signed-off-by: Takashi Sakamoto <[email protected]>
---
drivers/firewire/ohci.c | 49 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 7e88fd489741..62af3fa39a70 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -279,6 +279,8 @@ static char ohci_driver_name[] = KBUILD_MODNAME;
#define QUIRK_TI_SLLZ059 0x20
#define QUIRK_IR_WAKE 0x40

+#define QUIRK_REBOOT_BY_CYCLE_TIMER_READ 0x80000000
+
/* In case of multiple matches in ohci_quirks[], only the first one is used. */
static const struct {
unsigned short vendor, device, revision, flags;
@@ -1724,6 +1726,11 @@ static u32 get_cycle_time(struct fw_ohci *ohci)
s32 diff01, diff12;
int i;

+#if IS_ENABLED(CONFIG_X86)
+ if (ohci->quirks & QUIRK_REBOOT_BY_CYCLE_TIMER_READ)
+ return 0;
+#endif
+
c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer);

if (ohci->quirks & QUIRK_CYCLE_TIMER) {
@@ -3527,6 +3534,45 @@ static const struct fw_card_driver ohci_driver = {
.stop_iso = ohci_stop_iso,
};

+// On PCI Express Root Complex in any type of AMD Ryzen machine, VIA VT6306/6307/6308 with Asmedia
+// ASM1083/1085 brings an inconvenience that read accesses to 'Isochronous Cycle Timer' register
+// (at offset 0xf0 in PCI I/O space) often causes unexpected system reboot. The mechanism is not
+// clear, since the read access to the other registers is enough safe; e.g. 'Node ID' register,
+// while it is probable due to detection of any type of PCIe error.
+#if IS_ENABLED(CONFIG_X86)
+
+#define PCI_DEVICE_ID_ASMEDIA_ASM108X 0x1080
+
+static bool detect_vt630x_with_asm1083_on_amd_ryzen_machine(const struct pci_dev *pdev,
+ struct fw_ohci *ohci)
+{
+ const struct pci_dev *pcie_to_pci_bridge;
+ const struct cpuinfo_x86 *cinfo = &cpu_data(0);
+
+ // Detect any type of AMD Ryzen machine.
+ if (cinfo->x86_vendor != X86_VENDOR_AMD || cinfo->x86 < 0x17)
+ return false;
+
+ // Detect VIA VT6306/6307/6308.
+ if (pdev->vendor != PCI_VENDOR_ID_VIA)
+ return false;
+ if (pdev->device != PCI_DEVICE_ID_VIA_VT630X)
+ return false;
+
+ // Detect Asmedia ASM1083/1085.
+ pcie_to_pci_bridge = pdev->bus->self;
+ if (pcie_to_pci_bridge->vendor != PCI_VENDOR_ID_ASMEDIA)
+ return false;
+ if (pcie_to_pci_bridge->device != PCI_DEVICE_ID_ASMEDIA_ASM108X)
+ return false;
+
+ return true;
+}
+
+#else
+#define detect_vt630x_with_asm1083_on_amd_ryzen_machine(pdev) false
+#endif
+
#ifdef CONFIG_PPC_PMAC
static void pmac_ohci_on(struct pci_dev *dev)
{
@@ -3630,6 +3676,9 @@ static int pci_probe(struct pci_dev *dev,
if (param_quirks)
ohci->quirks = param_quirks;

+ if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
+ ohci->quirks |= QUIRK_REBOOT_BY_CYCLE_TIMER_READ;
+
/*
* Because dma_alloc_coherent() allocates at least one page,
* we save space by using a common buffer for the AR request/
--
2.39.2



2023-12-29 22:57:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards

Hi Takashi,

kernel test robot noticed the following build errors:

[auto build test ERROR on ieee1394-linux1394/for-next]
[also build test ERROR on ieee1394-linux1394/for-linus linus/master v6.7-rc7 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Takashi-Sakamoto/firewire-ohci-suppress-unexpected-system-reboot-in-AMD-Ryzen-machines-and-ASM108x-VT630x-PCIe-cards/20231229-120311
base: https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git for-next
patch link: https://lore.kernel.org/r/20231229035735.11127-1-o-takashi%40sakamocchi.jp
patch subject: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards
config: loongarch-randconfig-r081-20231229 (https://download.01.org/0day-ci/archive/20231230/[email protected]/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231230/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/firewire/ohci.c: In function 'pci_probe':
>> drivers/firewire/ohci.c:3679:70: error: macro "detect_vt630x_with_asm1083_on_amd_ryzen_machine" passed 2 arguments, but takes just 1
3679 | if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
| ^
drivers/firewire/ohci.c:3573: note: macro "detect_vt630x_with_asm1083_on_amd_ryzen_machine" defined here
3573 | #define detect_vt630x_with_asm1083_on_amd_ryzen_machine(pdev) false
|
>> drivers/firewire/ohci.c:3679:13: error: 'detect_vt630x_with_asm1083_on_amd_ryzen_machine' undeclared (first use in this function)
3679 | if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/firewire/ohci.c:3679:13: note: each undeclared identifier is reported only once for each function it appears in


vim +/detect_vt630x_with_asm1083_on_amd_ryzen_machine +3679 drivers/firewire/ohci.c

3617
3618 static int pci_probe(struct pci_dev *dev,
3619 const struct pci_device_id *ent)
3620 {
3621 struct fw_ohci *ohci;
3622 u32 bus_options, max_receive, link_speed, version;
3623 u64 guid;
3624 int i, err;
3625 size_t size;
3626
3627 if (dev->vendor == PCI_VENDOR_ID_PINNACLE_SYSTEMS) {
3628 dev_err(&dev->dev, "Pinnacle MovieBoard is not yet supported\n");
3629 return -ENOSYS;
3630 }
3631
3632 ohci = devres_alloc(release_ohci, sizeof(*ohci), GFP_KERNEL);
3633 if (ohci == NULL)
3634 return -ENOMEM;
3635 fw_card_initialize(&ohci->card, &ohci_driver, &dev->dev);
3636 pci_set_drvdata(dev, ohci);
3637 pmac_ohci_on(dev);
3638 devres_add(&dev->dev, ohci);
3639
3640 err = pcim_enable_device(dev);
3641 if (err) {
3642 dev_err(&dev->dev, "failed to enable OHCI hardware\n");
3643 return err;
3644 }
3645
3646 pci_set_master(dev);
3647 pci_write_config_dword(dev, OHCI1394_PCI_HCI_Control, 0);
3648
3649 spin_lock_init(&ohci->lock);
3650 mutex_init(&ohci->phy_reg_mutex);
3651
3652 INIT_WORK(&ohci->bus_reset_work, bus_reset_work);
3653
3654 if (!(pci_resource_flags(dev, 0) & IORESOURCE_MEM) ||
3655 pci_resource_len(dev, 0) < OHCI1394_REGISTER_SIZE) {
3656 ohci_err(ohci, "invalid MMIO resource\n");
3657 return -ENXIO;
3658 }
3659
3660 err = pcim_iomap_regions(dev, 1 << 0, ohci_driver_name);
3661 if (err) {
3662 ohci_err(ohci, "request and map MMIO resource unavailable\n");
3663 return -ENXIO;
3664 }
3665 ohci->registers = pcim_iomap_table(dev)[0];
3666
3667 for (i = 0; i < ARRAY_SIZE(ohci_quirks); i++)
3668 if ((ohci_quirks[i].vendor == dev->vendor) &&
3669 (ohci_quirks[i].device == (unsigned short)PCI_ANY_ID ||
3670 ohci_quirks[i].device == dev->device) &&
3671 (ohci_quirks[i].revision == (unsigned short)PCI_ANY_ID ||
3672 ohci_quirks[i].revision >= dev->revision)) {
3673 ohci->quirks = ohci_quirks[i].flags;
3674 break;
3675 }
3676 if (param_quirks)
3677 ohci->quirks = param_quirks;
3678
> 3679 if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
3680 ohci->quirks |= QUIRK_REBOOT_BY_CYCLE_TIMER_READ;
3681
3682 /*
3683 * Because dma_alloc_coherent() allocates at least one page,
3684 * we save space by using a common buffer for the AR request/
3685 * response descriptors and the self IDs buffer.
3686 */
3687 BUILD_BUG_ON(AR_BUFFERS * sizeof(struct descriptor) > PAGE_SIZE/4);
3688 BUILD_BUG_ON(SELF_ID_BUF_SIZE > PAGE_SIZE/2);
3689 ohci->misc_buffer = dmam_alloc_coherent(&dev->dev, PAGE_SIZE, &ohci->misc_buffer_bus,
3690 GFP_KERNEL);
3691 if (!ohci->misc_buffer)
3692 return -ENOMEM;
3693
3694 err = ar_context_init(&ohci->ar_request_ctx, ohci, 0,
3695 OHCI1394_AsReqRcvContextControlSet);
3696 if (err < 0)
3697 return err;
3698
3699 err = ar_context_init(&ohci->ar_response_ctx, ohci, PAGE_SIZE/4,
3700 OHCI1394_AsRspRcvContextControlSet);
3701 if (err < 0)
3702 return err;
3703
3704 err = context_init(&ohci->at_request_ctx, ohci,
3705 OHCI1394_AsReqTrContextControlSet, handle_at_packet);
3706 if (err < 0)
3707 return err;
3708
3709 err = context_init(&ohci->at_response_ctx, ohci,
3710 OHCI1394_AsRspTrContextControlSet, handle_at_packet);
3711 if (err < 0)
3712 return err;
3713
3714 reg_write(ohci, OHCI1394_IsoRecvIntMaskSet, ~0);
3715 ohci->ir_context_channels = ~0ULL;
3716 ohci->ir_context_support = reg_read(ohci, OHCI1394_IsoRecvIntMaskSet);
3717 reg_write(ohci, OHCI1394_IsoRecvIntMaskClear, ~0);
3718 ohci->ir_context_mask = ohci->ir_context_support;
3719 ohci->n_ir = hweight32(ohci->ir_context_mask);
3720 size = sizeof(struct iso_context) * ohci->n_ir;
3721 ohci->ir_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL);
3722 if (!ohci->ir_context_list)
3723 return -ENOMEM;
3724
3725 reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0);
3726 ohci->it_context_support = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
3727 /* JMicron JMB38x often shows 0 at first read, just ignore it */
3728 if (!ohci->it_context_support) {
3729 ohci_notice(ohci, "overriding IsoXmitIntMask\n");
3730 ohci->it_context_support = 0xf;
3731 }
3732 reg_write(ohci, OHCI1394_IsoXmitIntMaskClear, ~0);
3733 ohci->it_context_mask = ohci->it_context_support;
3734 ohci->n_it = hweight32(ohci->it_context_mask);
3735 size = sizeof(struct iso_context) * ohci->n_it;
3736 ohci->it_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL);
3737 if (!ohci->it_context_list)
3738 return -ENOMEM;
3739
3740 ohci->self_id = ohci->misc_buffer + PAGE_SIZE/2;
3741 ohci->self_id_bus = ohci->misc_buffer_bus + PAGE_SIZE/2;
3742
3743 bus_options = reg_read(ohci, OHCI1394_BusOptions);
3744 max_receive = (bus_options >> 12) & 0xf;
3745 link_speed = bus_options & 0x7;
3746 guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) |
3747 reg_read(ohci, OHCI1394_GUIDLo);
3748
3749 if (!(ohci->quirks & QUIRK_NO_MSI))
3750 pci_enable_msi(dev);
3751 err = devm_request_irq(&dev->dev, dev->irq, irq_handler,
3752 pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name, ohci);
3753 if (err < 0) {
3754 ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq);
3755 goto fail_msi;
3756 }
3757
3758 err = fw_card_add(&ohci->card, max_receive, link_speed, guid);
3759 if (err)
3760 goto fail_msi;
3761
3762 version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
3763 ohci_notice(ohci,
3764 "added OHCI v%x.%x device as card %d, "
3765 "%d IR + %d IT contexts, quirks 0x%x%s\n",
3766 version >> 16, version & 0xff, ohci->card.index,
3767 ohci->n_ir, ohci->n_it, ohci->quirks,
3768 reg_read(ohci, OHCI1394_PhyUpperBound) ?
3769 ", physUB" : "");
3770
3771 return 0;
3772
3773 fail_msi:
3774 pci_disable_msi(dev);
3775
3776 return err;
3777 }
3778

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-30 02:30:16

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards

On 12/28/2023 21:57, Takashi Sakamoto wrote:
> VIA VT6306/6307/6308 provides PCI interface compliant to 1394 OHCI. When
> the hardware is combined with Asmedia ASM1083/1085 PCIe-to-PCI bus bridge,
> it appears that accesses to its 'Isochronous Cycle Timer' register (offset
> 0xf0 on PCI I/O space) often causes unexpected system reboot in any type
> of AMD Ryzen machine (both 0x17 and 0x19 families). It does not appears in
> the other type of machine (AMD pre-Ryzen machine, Intel machine, at least),
> or in the other OHCI 1394 hardware (e.g. Texas Instruments).
>
> The issue explicitly appears at a commit dcadfd7f7c74 ("firewire: core:
> use union for callback of transaction completion") added to v6.5 kernel.
> It changed 1394 OHCI driver to access to the register every time to
> dispatch local asynchronous transaction. However, the issue exists in
> older version of kernel as long as it runs in AMD Ryzen machine, since
> the access to the register is required to maintain bus time. It is not
> hard to imagine that users experience the unexpected system reboot when
> generating bus reset by plugging any devices in, or reading the register
> by time-aware application programs; e.g. audio sample processing.
>
> Well, this commit suppresses the system reboot in the combination of
> hardware. It avoids the access itself. As a result, the software stack can
> not provide the hardware time anymore to unit drivers, userspace
> applications, and nodes in the same IEEE 1394 bus. It brings apparent
> disadvantage since time-aware application programs require it, while
> time-unaware applications are available again; e.g. sbp2.
>
> Cc: [email protected]
> Reported-by: Jiri Slaby <[email protected]>
> Closes: https://bugzilla.suse.com/show_bug.cgi?id=1215436
> Reported-by: Mario Limonciello <[email protected]>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217994
> Reported-by: Tobias Gruetzmacher <[email protected]>
> Closes: https://sourceforge.net/p/linux1394/mailman/message/58711901/
> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2240973
> Closes: https://bugs.launchpad.net/linux/+bug/2043905
> Signed-off-by: Takashi Sakamoto <[email protected]>
> ---
> drivers/firewire/ohci.c | 49 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
> index 7e88fd489741..62af3fa39a70 100644
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -279,6 +279,8 @@ static char ohci_driver_name[] = KBUILD_MODNAME;
> #define QUIRK_TI_SLLZ059 0x20
> #define QUIRK_IR_WAKE 0x40
>
> +#define QUIRK_REBOOT_BY_CYCLE_TIMER_READ 0x80000000
> +
> /* In case of multiple matches in ohci_quirks[], only the first one is used. */
> static const struct {
> unsigned short vendor, device, revision, flags;
> @@ -1724,6 +1726,11 @@ static u32 get_cycle_time(struct fw_ohci *ohci)
> s32 diff01, diff12;
> int i;
>
> +#if IS_ENABLED(CONFIG_X86)
> + if (ohci->quirks & QUIRK_REBOOT_BY_CYCLE_TIMER_READ)
> + return 0;
> +#endif
> +
> c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer);
>
> if (ohci->quirks & QUIRK_CYCLE_TIMER) {
> @@ -3527,6 +3534,45 @@ static const struct fw_card_driver ohci_driver = {
> .stop_iso = ohci_stop_iso,
> };
>
> +// On PCI Express Root Complex in any type of AMD Ryzen machine, VIA VT6306/6307/6308 with Asmedia
> +// ASM1083/1085 brings an inconvenience that read accesses to 'Isochronous Cycle Timer' register
> +// (at offset 0xf0 in PCI I/O space) often causes unexpected system reboot. The mechanism is not
> +// clear, since the read access to the other registers is enough safe; e.g. 'Node ID' register,
> +// while it is probable due to detection of any type of PCIe error.
> +#if IS_ENABLED(CONFIG_X86)
> +
> +#define PCI_DEVICE_ID_ASMEDIA_ASM108X 0x1080
> +
> +static bool detect_vt630x_with_asm1083_on_amd_ryzen_machine(const struct pci_dev *pdev,
> + struct fw_ohci *ohci)
> +{
> + const struct pci_dev *pcie_to_pci_bridge;
> + const struct cpuinfo_x86 *cinfo = &cpu_data(0);
> +
> + // Detect any type of AMD Ryzen machine.
> + if (cinfo->x86_vendor != X86_VENDOR_AMD || cinfo->x86 < 0x17)
> + return false;

Maybe it's better to use X86_FEATURE_ZEN?

> +
> + // Detect VIA VT6306/6307/6308.
> + if (pdev->vendor != PCI_VENDOR_ID_VIA)
> + return false;
> + if (pdev->device != PCI_DEVICE_ID_VIA_VT630X)
> + return false;
> +
> + // Detect Asmedia ASM1083/1085.
> + pcie_to_pci_bridge = pdev->bus->self;
> + if (pcie_to_pci_bridge->vendor != PCI_VENDOR_ID_ASMEDIA)
> + return false;
> + if (pcie_to_pci_bridge->device != PCI_DEVICE_ID_ASMEDIA_ASM108X)
> + return false;
> +
> + return true;
> +}
> +
> +#else
> +#define detect_vt630x_with_asm1083_on_amd_ryzen_machine(pdev) false
> +#endif
> +
> #ifdef CONFIG_PPC_PMAC
> static void pmac_ohci_on(struct pci_dev *dev)
> {
> @@ -3630,6 +3676,9 @@ static int pci_probe(struct pci_dev *dev,
> if (param_quirks)
> ohci->quirks = param_quirks;
>
> + if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
> + ohci->quirks |= QUIRK_REBOOT_BY_CYCLE_TIMER_READ;
> +
> /*
> * Because dma_alloc_coherent() allocates at least one page,
> * we save space by using a common buffer for the AR request/


2023-12-30 08:44:52

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards

Hi Takashi,

kernel test robot noticed the following build errors:

[auto build test ERROR on ieee1394-linux1394/for-next]
[also build test ERROR on ieee1394-linux1394/for-linus linus/master v6.7-rc7 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Takashi-Sakamoto/firewire-ohci-suppress-unexpected-system-reboot-in-AMD-Ryzen-machines-and-ASM108x-VT630x-PCIe-cards/20231229-120311
base: https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git for-next
patch link: https://lore.kernel.org/r/20231229035735.11127-1-o-takashi%40sakamocchi.jp
patch subject: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231230/[email protected]/config)
compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project 8a4266a626914765c0c69839e8a51be383013c1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231230/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/firewire/ohci.c:3679:59: error: too many arguments provided to function-like macro invocation
3679 | if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
| ^
drivers/firewire/ohci.c:3573:9: note: macro 'detect_vt630x_with_asm1083_on_amd_ryzen_machine' defined here
3573 | #define detect_vt630x_with_asm1083_on_amd_ryzen_machine(pdev) false
| ^
>> drivers/firewire/ohci.c:3679:6: error: use of undeclared identifier 'detect_vt630x_with_asm1083_on_amd_ryzen_machine'
3679 | if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
| ^
2 errors generated.


vim +3679 drivers/firewire/ohci.c

3617
3618 static int pci_probe(struct pci_dev *dev,
3619 const struct pci_device_id *ent)
3620 {
3621 struct fw_ohci *ohci;
3622 u32 bus_options, max_receive, link_speed, version;
3623 u64 guid;
3624 int i, err;
3625 size_t size;
3626
3627 if (dev->vendor == PCI_VENDOR_ID_PINNACLE_SYSTEMS) {
3628 dev_err(&dev->dev, "Pinnacle MovieBoard is not yet supported\n");
3629 return -ENOSYS;
3630 }
3631
3632 ohci = devres_alloc(release_ohci, sizeof(*ohci), GFP_KERNEL);
3633 if (ohci == NULL)
3634 return -ENOMEM;
3635 fw_card_initialize(&ohci->card, &ohci_driver, &dev->dev);
3636 pci_set_drvdata(dev, ohci);
3637 pmac_ohci_on(dev);
3638 devres_add(&dev->dev, ohci);
3639
3640 err = pcim_enable_device(dev);
3641 if (err) {
3642 dev_err(&dev->dev, "failed to enable OHCI hardware\n");
3643 return err;
3644 }
3645
3646 pci_set_master(dev);
3647 pci_write_config_dword(dev, OHCI1394_PCI_HCI_Control, 0);
3648
3649 spin_lock_init(&ohci->lock);
3650 mutex_init(&ohci->phy_reg_mutex);
3651
3652 INIT_WORK(&ohci->bus_reset_work, bus_reset_work);
3653
3654 if (!(pci_resource_flags(dev, 0) & IORESOURCE_MEM) ||
3655 pci_resource_len(dev, 0) < OHCI1394_REGISTER_SIZE) {
3656 ohci_err(ohci, "invalid MMIO resource\n");
3657 return -ENXIO;
3658 }
3659
3660 err = pcim_iomap_regions(dev, 1 << 0, ohci_driver_name);
3661 if (err) {
3662 ohci_err(ohci, "request and map MMIO resource unavailable\n");
3663 return -ENXIO;
3664 }
3665 ohci->registers = pcim_iomap_table(dev)[0];
3666
3667 for (i = 0; i < ARRAY_SIZE(ohci_quirks); i++)
3668 if ((ohci_quirks[i].vendor == dev->vendor) &&
3669 (ohci_quirks[i].device == (unsigned short)PCI_ANY_ID ||
3670 ohci_quirks[i].device == dev->device) &&
3671 (ohci_quirks[i].revision == (unsigned short)PCI_ANY_ID ||
3672 ohci_quirks[i].revision >= dev->revision)) {
3673 ohci->quirks = ohci_quirks[i].flags;
3674 break;
3675 }
3676 if (param_quirks)
3677 ohci->quirks = param_quirks;
3678
> 3679 if (detect_vt630x_with_asm1083_on_amd_ryzen_machine(dev, ohci))
3680 ohci->quirks |= QUIRK_REBOOT_BY_CYCLE_TIMER_READ;
3681
3682 /*
3683 * Because dma_alloc_coherent() allocates at least one page,
3684 * we save space by using a common buffer for the AR request/
3685 * response descriptors and the self IDs buffer.
3686 */
3687 BUILD_BUG_ON(AR_BUFFERS * sizeof(struct descriptor) > PAGE_SIZE/4);
3688 BUILD_BUG_ON(SELF_ID_BUF_SIZE > PAGE_SIZE/2);
3689 ohci->misc_buffer = dmam_alloc_coherent(&dev->dev, PAGE_SIZE, &ohci->misc_buffer_bus,
3690 GFP_KERNEL);
3691 if (!ohci->misc_buffer)
3692 return -ENOMEM;
3693
3694 err = ar_context_init(&ohci->ar_request_ctx, ohci, 0,
3695 OHCI1394_AsReqRcvContextControlSet);
3696 if (err < 0)
3697 return err;
3698
3699 err = ar_context_init(&ohci->ar_response_ctx, ohci, PAGE_SIZE/4,
3700 OHCI1394_AsRspRcvContextControlSet);
3701 if (err < 0)
3702 return err;
3703
3704 err = context_init(&ohci->at_request_ctx, ohci,
3705 OHCI1394_AsReqTrContextControlSet, handle_at_packet);
3706 if (err < 0)
3707 return err;
3708
3709 err = context_init(&ohci->at_response_ctx, ohci,
3710 OHCI1394_AsRspTrContextControlSet, handle_at_packet);
3711 if (err < 0)
3712 return err;
3713
3714 reg_write(ohci, OHCI1394_IsoRecvIntMaskSet, ~0);
3715 ohci->ir_context_channels = ~0ULL;
3716 ohci->ir_context_support = reg_read(ohci, OHCI1394_IsoRecvIntMaskSet);
3717 reg_write(ohci, OHCI1394_IsoRecvIntMaskClear, ~0);
3718 ohci->ir_context_mask = ohci->ir_context_support;
3719 ohci->n_ir = hweight32(ohci->ir_context_mask);
3720 size = sizeof(struct iso_context) * ohci->n_ir;
3721 ohci->ir_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL);
3722 if (!ohci->ir_context_list)
3723 return -ENOMEM;
3724
3725 reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0);
3726 ohci->it_context_support = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
3727 /* JMicron JMB38x often shows 0 at first read, just ignore it */
3728 if (!ohci->it_context_support) {
3729 ohci_notice(ohci, "overriding IsoXmitIntMask\n");
3730 ohci->it_context_support = 0xf;
3731 }
3732 reg_write(ohci, OHCI1394_IsoXmitIntMaskClear, ~0);
3733 ohci->it_context_mask = ohci->it_context_support;
3734 ohci->n_it = hweight32(ohci->it_context_mask);
3735 size = sizeof(struct iso_context) * ohci->n_it;
3736 ohci->it_context_list = devm_kzalloc(&dev->dev, size, GFP_KERNEL);
3737 if (!ohci->it_context_list)
3738 return -ENOMEM;
3739
3740 ohci->self_id = ohci->misc_buffer + PAGE_SIZE/2;
3741 ohci->self_id_bus = ohci->misc_buffer_bus + PAGE_SIZE/2;
3742
3743 bus_options = reg_read(ohci, OHCI1394_BusOptions);
3744 max_receive = (bus_options >> 12) & 0xf;
3745 link_speed = bus_options & 0x7;
3746 guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) |
3747 reg_read(ohci, OHCI1394_GUIDLo);
3748
3749 if (!(ohci->quirks & QUIRK_NO_MSI))
3750 pci_enable_msi(dev);
3751 err = devm_request_irq(&dev->dev, dev->irq, irq_handler,
3752 pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name, ohci);
3753 if (err < 0) {
3754 ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq);
3755 goto fail_msi;
3756 }
3757
3758 err = fw_card_add(&ohci->card, max_receive, link_speed, guid);
3759 if (err)
3760 goto fail_msi;
3761
3762 version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
3763 ohci_notice(ohci,
3764 "added OHCI v%x.%x device as card %d, "
3765 "%d IR + %d IT contexts, quirks 0x%x%s\n",
3766 version >> 16, version & 0xff, ohci->card.index,
3767 ohci->n_ir, ohci->n_it, ohci->quirks,
3768 reg_read(ohci, OHCI1394_PhyUpperBound) ?
3769 ", physUB" : "");
3770
3771 return 0;
3772
3773 fail_msi:
3774 pci_disable_msi(dev);
3775
3776 return err;
3777 }
3778

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-01-02 03:55:37

by Takashi Sakamoto

[permalink] [raw]
Subject: Re: [PATCH] firewire: ohci: suppress unexpected system reboot in AMD Ryzen machines and ASM108x/VT630x PCIe cards

Hi Mario,

On Fri, Dec 29, 2023 at 08:30:00PM -0600, Mario Limonciello wrote:
> On 12/28/2023 21:57, Takashi Sakamoto wrote:
> > @@ -3527,6 +3534,45 @@ static const struct fw_card_driver ohci_driver = {
> > .stop_iso = ohci_stop_iso,
> > };
> > +// On PCI Express Root Complex in any type of AMD Ryzen machine, VIA VT6306/6307/6308 with Asmedia
> > +// ASM1083/1085 brings an inconvenience that read accesses to 'Isochronous Cycle Timer' register
> > +// (at offset 0xf0 in PCI I/O space) often causes unexpected system reboot. The mechanism is not
> > +// clear, since the read access to the other registers is enough safe; e.g. 'Node ID' register,
> > +// while it is probable due to detection of any type of PCIe error.
> > +#if IS_ENABLED(CONFIG_X86)
> > +
> > +#define PCI_DEVICE_ID_ASMEDIA_ASM108X 0x1080
> > +
> > +static bool detect_vt630x_with_asm1083_on_amd_ryzen_machine(const struct pci_dev *pdev,
> > + struct fw_ohci *ohci)
> > +{
> > + const struct pci_dev *pcie_to_pci_bridge;
> > + const struct cpuinfo_x86 *cinfo = &cpu_data(0);
> > +
> > + // Detect any type of AMD Ryzen machine.
> > + if (cinfo->x86_vendor != X86_VENDOR_AMD || cinfo->x86 < 0x17)
> > + return false;
>
> Maybe it's better to use X86_FEATURE_ZEN?

Indeed. We can use it under the condition branch for CONFIG_X86, like:

+ // Detect any type of AMD Ryzen machine.
+ if (!static_cpu_has(X86_FEATURE_ZEN))
+ return false;


Thanks

Takashi Sakamoto