Skip to content

lib/pem: fix SDP queue mapping #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anoobj wants to merge 1 commit into
base: dao-devel
Choose a base branch
from
Open

lib/pem: fix SDP queue mapping #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/pem/pem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ dao_pem_host_interrupt_setup(uint16_t pem_devid, int vfid, uint64_t **intr_addr)
uint64_t reg_val;
uint8_t rpvf;

reg_val = sdp_reg_read(&pem->sdp_pdev, SDP_VF_MBOX_DATA(0));
sdp_reg_read(&pem->sdp_pdev, SDP_VF_MBOX_DATA(0), &reg_val);
rpvf = (reg_val >> SDP_EPFX_RINFO_RPVF_SHIFT) & 0xf;

if (!rpvf) {
Expand Down
34 changes: 21 additions & 13 deletions lib/pem/sdp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,35 @@
#define SDP0_PCIE_DEV_NAME "0002:18:00.0"
#define SDP1_PCIE_DEV_NAME "0002:19:00.0"

static inline int sdp_offset_valid(struct dao_vfio_device *sdp_pdev, uint64_t offset)
Copy link

@sourcery-ai sourcery-ai bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): sdp_offset_valid dereferences sdp_pdev without verifying it's non-null

Please add a null check for sdp_pdev or document that it must not be null.

{
return offset < sdp_pdev->mem[DAO_VFIO_DEV_BAR2].len;
Copy link

@sourcery-ai sourcery-ai bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Offset validation doesn't account for full 8-byte access width

Validate that offset + sizeof(uint64_t) <= len to prevent out-of-bounds access when performing 8-byte operations.

}

int
sdp_reg_write(struct dao_vfio_device *sdp_pdev, uint64_t offset, uint64_t val)
{
if (offset > sdp_pdev->mem[DAO_VFIO_DEV_BAR2].len)
if (!sdp_offset_valid(sdp_pdev, offset))
return -ENOMEM;
Comment on lines +28 to 29
Copy link

@sourcery-ai sourcery-ai bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Use a more appropriate error code than -ENOMEM for invalid offsets

Consider returning -EINVAL or -ERANGE instead to better indicate an invalid offset.

Suggested change
if (!sdp_offset_valid(sdp_pdev, offset))
return -ENOMEM;
if (!sdp_offset_valid(sdp_pdev, offset))
return -EINVAL;


*((volatile uint64_t *)(sdp_pdev->mem[DAO_VFIO_DEV_BAR2].addr + offset)) = val;
return 0;
}

uint64_t
sdp_reg_read(struct dao_vfio_device *sdp_pdev, uint64_t offset)
int
sdp_reg_read(struct dao_vfio_device *sdp_pdev, uint64_t offset, uint64_t *val)
{
if (offset > sdp_pdev->mem[DAO_VFIO_DEV_BAR2].len)
if (!sdp_pdev || !val || !sdp_offset_valid(sdp_pdev, offset))
return -ENOMEM;

return *(volatile uint64_t *)(sdp_pdev->mem[DAO_VFIO_DEV_BAR2].addr + offset);
*val = *(volatile uint64_t *)(sdp_pdev->mem[DAO_VFIO_DEV_BAR2].addr + offset);
return 0;
}

uint64_t *
sdp_reg_addr(struct dao_vfio_device *sdp_pdev, uint64_t offset)
{
if (offset > sdp_pdev->mem[DAO_VFIO_DEV_BAR2].len)
if (!sdp_offset_valid(sdp_pdev, offset))
return NULL;

return (uint64_t *)(sdp_pdev->mem[DAO_VFIO_DEV_BAR2].addr + offset);
Comment on lines +48 to 51
Copy link

@sourcery-ai sourcery-ai bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Potential unaligned pointer cast for register address

Enforce alignment by checking that offset is a multiple of sizeof(uint64_t), or use memcpy for unaligned access.

Suggested change
if (!sdp_offset_valid(sdp_pdev, offset))
return NULL;
return (uint64_t *)(sdp_pdev->mem[DAO_VFIO_DEV_BAR2].addr + offset);
if (!sdp_offset_valid(sdp_pdev, offset))
return NULL;
if (offset % sizeof(uint64_t) != 0)
return NULL;
return (uint64_t *)(sdp_pdev->mem[DAO_VFIO_DEV_BAR2].addr + offset);

Expand All @@ -66,14 +72,14 @@ sdp_init(struct dao_vfio_device *sdp_pdev)
sdp_pdev->mbar = DAO_VFIO_DEV_BAR4;

if (sdp_pdev->prime) {
reg_val = sdp_reg_read(sdp_pdev, SDP_EPFX_RINFO(0));
sdp_reg_read(sdp_pdev, SDP_EPFX_RINFO(0), &reg_val);
Copy link

@sourcery-ai sourcery-ai bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Return code of sdp_reg_read is ignored in initialization

Please check the return value of sdp_reg_read and handle errors to prevent use of an uninitialized reg_val.

reg_val &= ~SDP_EPFX_RINFO_SRN_MASK;
sdp_reg_write(sdp_pdev, SDP_EPFX_RINFO(0), reg_val);
reg_val = sdp_reg_read(sdp_pdev, SDP_EPFX_RINFO(0));
sdp_reg_read(sdp_pdev, SDP_EPFX_RINFO(0), &reg_val);
rpvf = (reg_val >> SDP_EPFX_RINFO_RPVF_SHIFT) & 0xf;
num_vfs = (reg_val >> SDP_EPFX_RINFO_RPVF_SHIFT) & 0x7f;
num_vfs = (reg_val >> SDP_EPFX_RINFO_NVVF_SHIFT) & 0x7f;
/* Disable PF Ring */
reg_val = sdp_reg_read(sdp_pdev, SDP_MAC0_PF_RING_CTL);
sdp_reg_read(sdp_pdev, SDP_MAC0_PF_RING_CTL, &reg_val);
reg_val &= ~SDP_MAC0_PF_RING_CTL_RPPF_MASK;
sdp_reg_write(sdp_pdev, SDP_MAC0_PF_RING_CTL, reg_val);

Expand All @@ -90,9 +96,11 @@ sdp_init(struct dao_vfio_device *sdp_pdev)
info <<= 32;
sdp_reg_write(sdp_pdev, SDP_PF_MBOX_DATA(0), info);
vfid = num_vfs >> 1;
info = rpvf | ((uint64_t)vfid << 8) | ((uint64_t)num_vfs << 16);
info <<= 32;
sdp_reg_write(sdp_pdev, SDP_PF_MBOX_DATA(32), info);
if (vfid) {
info = rpvf | ((uint64_t)vfid << 8) | ((uint64_t)num_vfs << 16);
info <<= 32;
sdp_reg_write(sdp_pdev, SDP_PF_MBOX_DATA(vfid * rpvf), info);
}
}

return 0;
Expand Down
2 changes: 1 addition & 1 deletion lib/pem/sdp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#define SDP_EPFX_RINFO_SRN_MASK DAO_GENMASK_ULL(6, 0)

int sdp_init(struct dao_vfio_device *sdp_pdev);
uint64_t sdp_reg_read(struct dao_vfio_device *sdp_pdev, uint64_t offset);
int sdp_reg_read(struct dao_vfio_device *sdp_pdev, uint64_t offset, uint64_t *val);
int sdp_reg_write(struct dao_vfio_device *sdp_pdev, uint64_t offset, uint64_t val);
uint64_t *sdp_reg_addr(struct dao_vfio_device *sdp_pdev, uint64_t offset);
void sdp_fini(struct dao_vfio_device *sdp_pdev);
Expand Down