Hi Cédric,
> > > +{
> > > + AspeedSBCState *s = ASPEED_SBC(opaque);
> > > + uint32_t otp_addr, data, otp_offset;
> > > + bool is_data = false;
> > > + Error *local_err = NULL;
> > > +
> > > + assert(s->otpmem);
> > > +
> > > + otp_addr = s->regs[R_ADDR];
> > > + if (otp_addr < OTP_DATA_DWORD_COUNT) {
> > > + is_data = true;
> > > + } else if (otp_addr >= OTP_TOTAL_DWORD_COUNT) {
> > > + qemu_log_mask(LOG_GUEST_ERROR,
> > > + "%s: Invalid OTP addr 0x%x\n",
> > > + __func__, otp_addr);
> > > + return;
> > > + }
> > > + otp_offset = otp_addr << 2;
> > > +
> > > + s->otpmem->ops->read(s->otpmem, otp_offset, &data, &local_err);
> > > + if (local_err) {
> > > + qemu_log_mask(LOG_GUEST_ERROR,
> > > + "%s: Failed to read data 0x%x, %s\n",
> > > + __func__, otp_offset,
> > > + error_get_pretty(local_err));
> > > + error_free(local_err);
> > > + return;> + }
> >
> > Please use an AddressSpace. See aspeed_smc for an example.
> >
> I will rework the code to use an AddressSpace for OTP memory access.
> Thanks for pointing me to the aspeed_smc example — it’s very helpful.
Sorry for bringing up this topic again. While working with AddressSpace,
I found that I couldn't directly use address_space_read() or
address_space_write() in the SBC code, because the OTP memory enforces
more specific access rules.
For example, some regions of the OTP memory may be configured as
write-only, meaning the guest is not allowed to read from them. To
support this behavior, I plan to implement custom read/write functions
in the OTP memory device. These functions will act as wrappers around
the AddressSpace APIs and enforce additional checks before allowing
memory accesses from the SBC.
These additional checks will be introduced in a separate patch to keep
the changes organized and easier to review.
Do you have any concerns about this approach?
Best Regards,
Kane
