On Wed, Jan 01, 2025 at 05:39:24PM +0200, Alexander Usyskin wrote: > Enable access to internal non-volatile memory on DGFX > with GSC/CSC devices via a child device. > The nvm child device is exposed via auxiliary bus. > > Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com>
Since this patch needs a rebase anyway, please allow me to point out a few nipticks that I'd like you to adjust. I'm sorry for not catching this earlier... A rebased version with my nipticks can be found here: https://github.com/rodrigovivi/linux/tree/nvm-mtd (But you need to add more platforms there... I just added the BMG, and also I didn't address all the items that I'm listing below) > Signed-off-by: Alexander Usyskin <alexander.usys...@intel.com> > --- > drivers/gpu/drm/xe/Makefile | 1 + > drivers/gpu/drm/xe/xe_device.c | 3 + > drivers/gpu/drm/xe/xe_device_types.h | 7 ++ > drivers/gpu/drm/xe/xe_nvm.c | 100 +++++++++++++++++++++++++++ > drivers/gpu/drm/xe/xe_nvm.h | 15 ++++ > drivers/gpu/drm/xe/xe_pci.c | 6 ++ > 6 files changed, 132 insertions(+) > create mode 100644 drivers/gpu/drm/xe/xe_nvm.c > create mode 100644 drivers/gpu/drm/xe/xe_nvm.h > > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile > index 7730e0596299..f5f3a3233fab 100644 > --- a/drivers/gpu/drm/xe/Makefile > +++ b/drivers/gpu/drm/xe/Makefile > @@ -94,6 +94,7 @@ xe-y += xe_bb.o \ > xe_ring_ops.o \ > xe_sa.o \ > xe_sched_job.o \ > + xe_nvm.o \ move up to respect alphabetical order. > xe_step.o \ > xe_sync.o \ > xe_tile.o \ > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c > index 7f021ec5f8e7..78e5e88435c6 100644 > --- a/drivers/gpu/drm/xe/xe_device.c > +++ b/drivers/gpu/drm/xe/xe_device.c > @@ -50,6 +50,7 @@ > #include "xe_pcode.h" > #include "xe_pm.h" > #include "xe_query.h" > +#include "xe_nvm.h" > #include "xe_sriov.h" > #include "xe_tile.h" > #include "xe_ttm_stolen_mgr.h" > @@ -743,6 +744,7 @@ int xe_device_probe(struct xe_device *xe) > goto err_fini_gt; > } > > + xe_nvm_init(xe); > xe_heci_gsc_init(xe); > > err = xe_oa_init(xe); > @@ -813,6 +815,7 @@ void xe_device_remove(struct xe_device *xe) > xe_oa_fini(xe); > > xe_heci_gsc_fini(xe); > + xe_nvm_fini(xe); > > for_each_gt(gt, xe, id) > xe_gt_remove(gt); > diff --git a/drivers/gpu/drm/xe/xe_device_types.h > b/drivers/gpu/drm/xe/xe_device_types.h > index 8a7b15972413..817c8fac9d9d 100644 > --- a/drivers/gpu/drm/xe/xe_device_types.h > +++ b/drivers/gpu/drm/xe/xe_device_types.h > @@ -35,6 +35,8 @@ > struct xe_ggtt; > struct xe_pat_ops; > > +struct intel_dg_nvm_dev; move above xe_ggtt and no space needed. > + > #define XE_BO_INVALID_OFFSET LONG_MAX > > #define GRAPHICS_VER(xe) ((xe)->info.graphics_verx100 / 100) > @@ -308,6 +310,8 @@ struct xe_device { > u8 has_device_atomics_on_smem:1; > /** @info.has_flat_ccs: Whether flat CCS metadata is used */ > u8 has_flat_ccs:1; > + /** @info.has_gsc_nvm: device has gsc non-volatile memory */ > + u8 has_gsc_nvm:1; > /** @info.has_heci_cscfi: device has heci cscfi */ > u8 has_heci_cscfi:1; > /** @info.has_heci_gscfi: device has heci gscfi */ > @@ -511,6 +515,9 @@ struct xe_device { > /** @heci_gsc: graphics security controller */ > struct xe_heci_gsc heci_gsc; > > + /** @nvm: discrete graphics non-volatile memory */ > + struct intel_dg_nvm_dev *nvm; > + > /** @oa: oa observation subsystem */ > struct xe_oa oa; > > diff --git a/drivers/gpu/drm/xe/xe_nvm.c b/drivers/gpu/drm/xe/xe_nvm.c > new file mode 100644 > index 000000000000..16383cbc9e1d > --- /dev/null > +++ b/drivers/gpu/drm/xe/xe_nvm.c > @@ -0,0 +1,100 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright(c) 2019-2024, Intel Corporation. All rights reserved. 2025 > + */ > + > +#include <linux/intel_dg_nvm_aux.h> > +#include <linux/pci.h> add a line here to separate these 2 blocks > +#include "xe_device_types.h" > +#include "xe_nvm.h" > +#include "xe_sriov.h" > + > +#define GEN12_GUNIT_NVM_BASE 0x00102040 > +#define GEN12_GUNIT_NVM_SIZE 0x80 > +#define HECI_FW_STATUS_2_NVM_ACCESS_MODE BIT(3) > + > +static const struct intel_dg_nvm_region regions[INTEL_DG_NVM_REGIONS] = { > + [0] = { .name = "DESCRIPTOR", }, > + [2] = { .name = "GSC", }, > + [11] = { .name = "OptionROM", }, > + [12] = { .name = "DAM", }, > +}; > + > +static void xe_nvm_release_dev(struct device *dev) > +{ > +} > + > +void xe_nvm_init(struct xe_device *xe) > +{ > + struct pci_dev *pdev = to_pci_dev(xe->drm.dev); > + struct intel_dg_nvm_dev *nvm; > + struct auxiliary_device *aux_dev; > + int ret; > + > + if (!xe->info.has_gsc_nvm) > + return; > + > + /* No access to internal NVM from VFs */ > + if (IS_SRIOV_VF(xe)) > + return; > + > + /* Nvm pointer should be NULL here */ > + if (WARN_ON(xe->nvm)) > + return; > + > + xe->nvm = kzalloc(sizeof(*nvm), GFP_KERNEL); > + if (!xe->nvm) > + return; > + > + nvm = xe->nvm; > + > + nvm->writeable_override = false; > + nvm->bar.parent = &pdev->resource[0]; > + nvm->bar.start = GEN12_GUNIT_NVM_BASE + pdev->resource[0].start; > + nvm->bar.end = nvm->bar.start + GEN12_GUNIT_NVM_SIZE - 1; > + nvm->bar.flags = IORESOURCE_MEM; > + nvm->bar.desc = IORES_DESC_NONE; > + nvm->regions = regions; > + > + aux_dev = &nvm->aux_dev; > + > + aux_dev->name = "nvm"; > + aux_dev->id = (pci_domain_nr(pdev->bus) << 16) | > + PCI_DEVID(pdev->bus->number, pdev->devfn); > + aux_dev->dev.parent = &pdev->dev; > + aux_dev->dev.release = xe_nvm_release_dev; > + > + ret = auxiliary_device_init(aux_dev); > + if (ret) { > + drm_err(&xe->drm, "xe-nvm aux init failed %d\n", ret); > + return; > + } > + > + ret = auxiliary_device_add(aux_dev); > + if (ret) { > + drm_err(&xe->drm, "xe-nvm aux add failed %d\n", ret); > + auxiliary_device_uninit(aux_dev); > + return; > + } > +} > + > +void xe_nvm_fini(struct xe_device *xe) > +{ > + struct intel_dg_nvm_dev *nvm = xe->nvm; > + > + if (!xe->info.has_gsc_nvm) > + return; > + > + /* No access to internal NVM from VFs */ > + if (IS_SRIOV_VF(xe)) > + return; > + > + /* Nvm pointer should not be NULL here */ > + if (WARN_ON(!nvm)) > + return; > + > + auxiliary_device_delete(&nvm->aux_dev); > + auxiliary_device_uninit(&nvm->aux_dev); > + kfree(nvm); > + xe->nvm = NULL; > +} > diff --git a/drivers/gpu/drm/xe/xe_nvm.h b/drivers/gpu/drm/xe/xe_nvm.h > new file mode 100644 > index 000000000000..068695447913 > --- /dev/null > +++ b/drivers/gpu/drm/xe/xe_nvm.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright(c) 2019-2024 Intel Corporation. All rights reserved. > + */ > + > +#ifndef __XE_NVM_H__ > +#define __XE_NVM_H__ > + > +struct xe_device; > + > +void xe_nvm_init(struct xe_device *xe); > + > +void xe_nvm_fini(struct xe_device *xe); > + > +#endif /* __XE_NVM_H__ */ #endif is enough here without the comment... > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c > index 6b7f77425c7f..9b8ce6397a67 100644 > --- a/drivers/gpu/drm/xe/xe_pci.c > +++ b/drivers/gpu/drm/xe/xe_pci.c > @@ -60,6 +60,7 @@ struct xe_device_desc { > u8 has_display:1; > u8 has_heci_gscfi:1; > u8 has_heci_cscfi:1; > + u8 has_gsc_nvm:1; move above heci ones to respect alphabetical order > u8 has_llc:1; > u8 has_mmio_ext:1; > u8 has_sriov:1; > @@ -282,6 +283,7 @@ static const struct xe_device_desc dg1_desc = { > PLATFORM(DG1), > .has_display = true, > .has_heci_gscfi = 1, > + .has_gsc_nvm = 1, move above heci one to respect alphabetical order > .require_force_probe = true, > }; > > @@ -293,6 +295,7 @@ static const u16 dg2_g12_ids[] = { > INTEL_DG2_G12_IDS(NOP), 0 }; > DGFX_FEATURES, \ > PLATFORM(DG2), \ > .has_heci_gscfi = 1, \ > + .has_gsc_nvm = 1, \ move above heci one to respect alphabetical order > .subplatforms = (const struct xe_subplatform_desc[]) { \ > { XE_SUBPLATFORM_DG2_G10, "G10", dg2_g10_ids }, \ > { XE_SUBPLATFORM_DG2_G11, "G11", dg2_g11_ids }, \ > @@ -324,6 +327,7 @@ static const __maybe_unused struct xe_device_desc > pvc_desc = { > PLATFORM(PVC), > .has_display = false, > .has_heci_gscfi = 1, > + .has_gsc_nvm = 1, move above heci one to respect alphabetical order > .require_force_probe = true, > }; > > @@ -344,6 +348,7 @@ static const struct xe_device_desc bmg_desc = { > PLATFORM(BATTLEMAGE), > .has_display = true, > .has_heci_cscfi = 1, > + .has_gsc_nvm = 1, move above heci one to respect alphabetical order > }; > > static const struct xe_device_desc ptl_desc = { > @@ -616,6 +621,7 @@ static int xe_info_init_early(struct xe_device *xe, > xe->info.is_dgfx = desc->is_dgfx; > xe->info.has_heci_gscfi = desc->has_heci_gscfi; > xe->info.has_heci_cscfi = desc->has_heci_cscfi; > + xe->info.has_gsc_nvm = desc->has_gsc_nvm; move above heci ones to respect alphabetical order > xe->info.has_llc = desc->has_llc; > xe->info.has_mmio_ext = desc->has_mmio_ext; > xe->info.has_sriov = desc->has_sriov; > -- > 2.43.0 >
