Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for vfio/pci: Add support for opregion v2.0+ (rev4)
Hi, Lakshmi: 1. The test failed on version 4 while passed on version 3. The only difference between v3/v4 lies in the format indent issue :“Alignment should match open parenthesis ” 2. My patch only works in VFIO during a guest virtual machine is launched on VT-d gpu passthru. i,e my patch should not be called in common i915 tests. Can u help on this failure ? Thanks in advance. BR/Fred From: Patchwork Sent: Tuesday, March 2, 2021 2:50 PM To: Gao, Fred Cc: intel-gfx@lists.freedesktop.org Subject: ✗ Fi.CI.BAT: failure for vfio/pci: Add support for opregion v2.0+ (rev4) Patch Details Series: vfio/pci: Add support for opregion v2.0+ (rev4) URL: https://patchwork.freedesktop.org/series/84494/ State: failure Details: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/index.html CI Bug Log - changes from CI_DRM_9819 -> Patchwork_19740 Summary FAILURE Serious unknown changes coming with Patchwork_19740 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_19740, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/index.html Possible new issues Here are the unknown changes that may have been introduced in Patchwork_19740: IGT changes Possible regressions * igt@gem_exec_gttfill@basic: * fi-kbl-8809g: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9819/fi-kbl-8809g/igt@gem_exec_gttf...@basic.html> -> TIMEOUT<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-kbl-8809g/igt@gem_exec_gttf...@basic.html> +1 similar issue Known issues Here are the changes found in Patchwork_19740 that come from known issues: IGT changes Issues hit * igt@amdgpu/amd_basic@memory-alloc: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@amdgpu/amd_ba...@memory-alloc.html> (fdo#109315<https://bugs.freedesktop.org/show_bug.cgi?id=109315>) +17 similar issues * igt@debugfs_test@read_all_entries: * fi-tgl-y: PASS<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9819/fi-tgl-y/igt@debugfs_test@read_all_entries.html> -> DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-tgl-y/igt@debugfs_test@read_all_entries.html> (i915#402<https://gitlab.freedesktop.org/drm/intel/issues/402>) +2 similar issues * igt@gem_exec_fence@basic-busy@bcs0: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@gem_exec_fence@basic-b...@bcs0.html> (i915#1208<https://gitlab.freedesktop.org/drm/intel/issues/1208>) +1 similar issue * igt@gem_huc_copy@huc-copy: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@gem_huc_c...@huc-copy.html> (i915#2190<https://gitlab.freedesktop.org/drm/intel/issues/2190>) * igt@kms_chamelium@hdmi-hpd-fast: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@kms_chamel...@hdmi-hpd-fast.html> (i915#1004<https://gitlab.freedesktop.org/drm/intel/issues/1004>) +2 similar issues * igt@kms_chamelium@vga-edid-read: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@kms_chamel...@vga-edid-read.html> (fdo#109309<https://bugs.freedesktop.org/show_bug.cgi?id=109309>) +1 similar issue * igt@kms_force_connector_basic@force-load-detect: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@kms_force_connector_ba...@force-load-detect.html> (fdo#109285<https://bugs.freedesktop.org/show_bug.cgi?id=109285>) * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d: * fi-cml-u2: NOTRUN -> SKIP<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-cml-u2/igt@kms_pipe_crc_ba...@compare-crc-sanitycheck-pipe-d.html> (fdo#109278<https://bugs.freedesktop.org/show_bug.cgi?id=109278> / i915#533<https://gitlab.freedesktop.org/drm/intel/issues/533>) Possible fixes * igt@gem_linear_blits@basic: * fi-kbl-8809g: TIMEOUT<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9819/fi-kbl-8809g/igt@gem_linear_bl...@basic.html> (i915#2502<https://gitlab.freedesktop.org/drm/intel/issues/2502>) -> PASS<https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19740/fi-kbl-8809g/igt@gem_linear_bl...@basic.html> * igt@prime_self_import@basic-with_one_bo_two_files: * fi-tgl-y: DMESG-WARN<https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9819/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html> (i915#402<https://gitlab.freedesktop.org/drm/intel/issues/402>) -> PASS&l
Re: [Intel-gfx] [PATCH v4] vfio/pci: Add support for opregion v2.1+
Thank you for offering your valuable advice. Will send the updated version soon. > -Original Message- > From: Alex Williamson > Sent: Saturday, March 20, 2021 3:27 AM > To: Gao, Fred > Cc: k...@vger.kernel.org; intel-gfx@lists.freedesktop.org; Zhenyu Wang > ; Fonn, Swee Yee > Subject: Re: [PATCH v4] vfio/pci: Add support for opregion v2.1+ > > On Tue, 2 Mar 2021 21:02:20 +0800 > Fred Gao wrote: > > > Before opregion version 2.0 VBT data is stored in opregion mailbox #4, > > However, When VBT data exceeds 6KB size and cannot be within mailbox > > #4 starting from opregion v2.0+, Extended VBT region, next to > > opregion, is used to hold the VBT data, so the total size will be > > opregion size plus extended VBT region size. > > > > since opregion v2.0 with physical host VBT address should not be > > practically available for end user, it is not supported. > > > > Cc: Zhenyu Wang > > Signed-off-by: Swee Yee Fonn > > Signed-off-by: Fred Gao > > --- > > drivers/vfio/pci/vfio_pci_igd.c | 49 > > + > > 1 file changed, 49 insertions(+) > > > > diff --git a/drivers/vfio/pci/vfio_pci_igd.c > > b/drivers/vfio/pci/vfio_pci_igd.c index 53d97f459252..4edb8afcdbfc > > 100644 > > --- a/drivers/vfio/pci/vfio_pci_igd.c > > +++ b/drivers/vfio/pci/vfio_pci_igd.c > > @@ -21,6 +21,10 @@ > > #define OPREGION_SIZE (8 * 1024) > > #define OPREGION_PCI_ADDR 0xfc > > > > +#define OPREGION_RVDA 0x3ba > > +#define OPREGION_RVDS 0x3c2 > > +#define OPREGION_VERSION 0x16 > > + > > static size_t vfio_pci_igd_rw(struct vfio_pci_device *vdev, char __user > *buf, > > size_t count, loff_t *ppos, bool iswrite) { @@ - > 58,6 +62,7 > > @@ static int vfio_pci_igd_opregion_init(struct vfio_pci_device *vdev) > > u32 addr, size; > > void *base; > > int ret; > > + u16 version; > > > > ret = pci_read_config_dword(vdev->pdev, OPREGION_PCI_ADDR, > &addr); > > if (ret) > > @@ -83,6 +88,50 @@ static int vfio_pci_igd_opregion_init(struct > > vfio_pci_device *vdev) > > > > size *= 1024; /* In KB */ > > > > + /* > > +* Support opregion v2.1+ > > +* When VBT data exceeds 6KB size and cannot be within mailbox #4 > > s/#4/#4, then the/ > > > +* Extended VBT region, next to opregion, is used to hold the VBT > data. > > +* RVDA (Relative Address of VBT Data from Opregion Base) and > RVDS > > +* (VBT Data Size) from opregion structure member are used to hold > the > > +* address from region base and size of VBT data while RVDA/RVDS > > +* are not defined before opregion 2.0. > > +* > > +* opregion 2.0: rvda is the physical VBT address. > > Let's expand the comment to include why this is a problem to support > (virtualization of this register would be required in userspace) and why we're > choosing not to manipulate this into a 2.1+ table, which I think is both the > practical lack of v2.0 tables in use and any implicit dependencies software > may have on the OpRegion version. > > > +* > > +* opregion 2.1+: rvda is unsigned, relative offset from > > +* opregion base, and should never point within opregion. > > And for our purposes must exactly follow the base opregion to avoid > exposing unknown host memory to userspace, ie. provide a more descriptive > justification for the 2nd error condition below. > > > +*/ > > + version = le16_to_cpu(*(__le16 *)(base + OPREGION_VERSION)); > > + if (version >= 0x0200) { > > + u64 rvda; > > + u32 rvds; > > + > > + rvda = le64_to_cpu(*(__le64 *)(base + OPREGION_RVDA)); > > + rvds = le32_to_cpu(*(__le32 *)(base + OPREGION_RVDS)); > > + if (rvda && rvds) { > > + /* no support for opregion v2.0 with physical VBT > address */ > > + if (version == 0x0200) { > > + memunmap(base); > > + pci_err(vdev->pdev, > > + "IGD passthrough does not support > opregion\n" > > + "version 0x%x with physical rvda > 0x%llx\n", version, rvda); > > > Why do we need a new line midway through this log message? > > s/passthrough/assignment/ > > In testing the version you include the leading zero, do you also want that > leading zer
Re: [Intel-gfx] ✗ Fi.CI.DOCS: warning for vfio/pci: Add support for opregion v2.0+ (rev5)
Hi, Lakshmi: Can u help on this failure again, the only difference between version 5& 4 is the comments. Thanks in advance. > -Original Message- > From: Patchwork > Sent: Friday, March 26, 2021 5:05 AM > To: Gao, Fred > Cc: intel-gfx@lists.freedesktop.org > Subject: ✗ Fi.CI.DOCS: warning for vfio/pci: Add support for opregion v2.0+ > (rev5) > > == Series Details == > > Series: vfio/pci: Add support for opregion v2.0+ (rev5) > URL : https://patchwork.freedesktop.org/series/84494/ > State : warning > > == Summary == > > $ make htmldocs 2>&1 > /dev/null | grep i915 > ./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function > parameter or member 'ww' not described in 'i915_gem_shrink' > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function > parameter 'trampoline' description in 'intel_engine_cmd_parser' > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function > parameter or member 'jump_whitelist' not described in > 'intel_engine_cmd_parser' > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function > parameter or member 'shadow_map' not described in > 'intel_engine_cmd_parser' > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function > parameter or member 'batch_map' not described in > 'intel_engine_cmd_parser' > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function > parameter 'trampoline' description in 'intel_engine_cmd_parser' > /home/cidrm/kernel/Documentation/gpu/i915:22: ./drivers/gpu/drm/i915/i > ntel_runtime_pm.c:423: WARNING: Inline strong start-string without end- > string. > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.DOCS: warning for vfio/pci: Add support for opregion v2.0+ (rev5)
Thanks for the quick help. Test of Fi.CI.DOCS is still failed although we do not change any code in i915. > -Original Message- > From: Vudum, Lakshminarayana > Sent: Tuesday, March 30, 2021 12:20 AM > To: Gao, Fred ; intel-gfx@lists.freedesktop.org > Cc: Wang, Zhenyu Z > Subject: RE: ✗ Fi.CI.DOCS: warning for vfio/pci: Add support for opregion > v2.0+ (rev5) > > Failure is reported. It's now showing as success. > > -----Original Message- > From: Gao, Fred > Sent: Monday, March 29, 2021 12:12 AM > To: intel-gfx@lists.freedesktop.org; Vudum, Lakshminarayana > > Cc: Wang, Zhenyu Z > Subject: RE: ✗ Fi.CI.DOCS: warning for vfio/pci: Add support for opregion > v2.0+ (rev5) > > Hi, Lakshmi: > > Can u help on this failure again, > the only difference between version 5& 4 is the comments. > Thanks in advance. > > > -Original Message- > > From: Patchwork > > Sent: Friday, March 26, 2021 5:05 AM > > To: Gao, Fred > > Cc: intel-gfx@lists.freedesktop.org > > Subject: ✗ Fi.CI.DOCS: warning for vfio/pci: Add support for opregion > > v2.0+ > > (rev5) > > > > == Series Details == > > > > Series: vfio/pci: Add support for opregion v2.0+ (rev5) > > URL : https://patchwork.freedesktop.org/series/84494/ > > State : warning > > > > == Summary == > > > > $ make htmldocs 2>&1 > /dev/null | grep i915 > > ./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function > > parameter or member 'ww' not described in 'i915_gem_shrink' > > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess > > function parameter 'trampoline' description in 'intel_engine_cmd_parser' > > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function > > parameter or member 'jump_whitelist' not described in > > 'intel_engine_cmd_parser' > > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function > > parameter or member 'shadow_map' not described in > > 'intel_engine_cmd_parser' > > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function > > parameter or member 'batch_map' not described in > > 'intel_engine_cmd_parser' > > ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess > > function parameter 'trampoline' description in 'intel_engine_cmd_parser' > > > /home/cidrm/kernel/Documentation/gpu/i915:22: ./drivers/gpu/drm/i915/i > > ntel_runtime_pm.c:423: WARNING: Inline strong start-string without > > end- string. > > > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1] vfio/pci: Add support for opregion v2.0+
Thanks Alex for the timely review. > -Original Message- > From: Alex Williamson > Sent: Thursday, December 3, 2020 2:57 AM > To: Gao, Fred > Cc: k...@vger.kernel.org; intel-gfx@lists.freedesktop.org; Zhenyu Wang > ; Fonn, Swee Yee > Subject: Re: [PATCH v1] vfio/pci: Add support for opregion v2.0+ > > On Thu, 3 Dec 2020 01:12:49 +0800 > Fred Gao wrote: > > > When VBT data exceeds 6KB size and cannot be within mailbox #4 > > starting from opregion v2.0+, Extended VBT region, next to opregion, > > is used to hold the VBT data, so the total size will be opregion size > > plus extended VBT region size. > > > > For opregion 2.1+: since rvda is relative offset from opregion base, > > rvda as extended VBT start offset should be same as opregion size. > > > > For opregion 2.0: the only difference between opregion 2.0 and 2.1 is > > rvda addressing mode besides the version. since rvda is physical host > > VBT address and cannot be directly used in guest, it is faked into > > opregion 2.1's relative offset. > > > > Cc: Zhenyu Wang > > Signed-off-by: Swee Yee Fonn > > Signed-off-by: Fred Gao > > --- > > drivers/vfio/pci/vfio_pci_igd.c | 44 > > + > > 1 file changed, 44 insertions(+) > > > > diff --git a/drivers/vfio/pci/vfio_pci_igd.c > > b/drivers/vfio/pci/vfio_pci_igd.c index 53d97f459252..78919a289914 > > 100644 > > --- a/drivers/vfio/pci/vfio_pci_igd.c > > +++ b/drivers/vfio/pci/vfio_pci_igd.c > > @@ -21,6 +21,17 @@ > > #define OPREGION_SIZE (8 * 1024) > > #define OPREGION_PCI_ADDR 0xfc > > > > +/* > > + * opregion 2.0: rvda is the physical VBT address. > > What's rvda? What's VBT? Rvda is a struct member in opregion mailbox 3 , same definition in i915's struct opregion_asle. I,e Physical address of raw VBT data (v2.0) or Relative address from opregion (v2.1). VBT: video bios table , the data is stored in opregion mailbox 4 before opregion v2.0. After opregion v2.0+ , VBT data is larger than mailbox 4, so Extended VBT region, next to opregion is used to hold the data. > > + * > > + * opregion 2.1+: rvda is unsigned, relative offset from > > + * opregion base, and should never point within opregion. > > + */ > > +#define OPREGION_RDVA 0x3ba > > +#define OPREGION_RDVS 0x3c2 > > +#define OPREGION_VERSION 22 > > Why is this specified as decimal and the others in hex? This makes it seem > like the actual version rather than the offset of a version register. Yes, it is an offset, will redefine the opregion version offset in hex. > > + > > + > > static size_t vfio_pci_igd_rw(struct vfio_pci_device *vdev, char __user > *buf, > > size_t count, loff_t *ppos, bool iswrite) { @@ - > 58,6 +69,7 > > @@ static int vfio_pci_igd_opregion_init(struct vfio_pci_device *vdev) > > u32 addr, size; > > void *base; > > int ret; > > + u16 version; > > > > ret = pci_read_config_dword(vdev->pdev, OPREGION_PCI_ADDR, > &addr); > > if (ret) > > @@ -83,6 +95,38 @@ static int vfio_pci_igd_opregion_init(struct > > vfio_pci_device *vdev) > > > > size *= 1024; /* In KB */ > > > > + /* Support opregion v2.0+ */ > > + version = le16_to_cpu(*(__le16 *)(base + OPREGION_VERSION)); > > + if (version >= 0x0200) { > > + u64 rvda; > > + u32 rvds; > > + > > + rvda = le64_to_cpu(*(__le64 *)(base + OPREGION_RDVA)); > > + rvds = le32_to_cpu(*(__le32 *)(base + OPREGION_RDVS)); > > + if (rvda && rvds) { > > + u32 offset; > > + > > + if (version == 0x0200) > > + offset = (rvda - (u64)addr); > > Unnecessary outer () Thx, will remove in new patch. > > + else > > + offset = rvda; > > + > > + pci_WARN(vdev->pdev, offset != size, > > + "Extended VBT does not follow opregion !\n" > > + "opregion version 0x%x:offset 0x%x\n", > version, offset); > > + > > + if (version == 0x0200) { > > + /* opregion version v2.0 faked to v2.1 */ > > + *(__le16 *)(base + OPREGION_VERSION) = > > + cpu_to_le16(0x0201); > > + /* rvda faked to rela