On Tue, 26 Sept 2023 at 15:12, Vladimir Sementsov-Ogievskiy
<vsement...@yandex-team.ru> wrote:
>
> On 26.09.23 13:37, Peter Maydell wrote:
> > On Mon, 25 Sept 2023 at 20:41, Vladimir Sementsov-Ogievskiy
> > <vsement...@yandex-team.ru> wrote:
> >>
> >> Add a constant and clear assertion. The assertion also tells Coverity
> >> that we are not going to overflow the array.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> >> ---
> >> hw/i386/intel_iommu.c | 11 ++++++++---
> >> 1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >> index c0ce896668..2233dbe13a 100644
> >> --- a/hw/i386/intel_iommu.c
> >> +++ b/hw/i386/intel_iommu.c
> >> @@ -1028,12 +1028,17 @@ static dma_addr_t
> >> vtd_get_iova_pgtbl_base(IntelIOMMUState *s,
> >> * vtd_spte_rsvd 4k pages
> >> * vtd_spte_rsvd_large large pages
> >> */
> >> -static uint64_t vtd_spte_rsvd[5];
> >> -static uint64_t vtd_spte_rsvd_large[5];
> >> +#define VTD_SPTE_RSVD_LEN 5
> >> +static uint64_t vtd_spte_rsvd[VTD_SPTE_RSVD_LEN];
> >> +static uint64_t vtd_spte_rsvd_large[VTD_SPTE_RSVD_LEN];
> >>
> >> static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level)
> >> {
> >> - uint64_t rsvd_mask = vtd_spte_rsvd[level];
> >> + uint64_t rsvd_mask;
> >> +
> >> + assert(level < VTD_SPTE_RSVD_LEN);
> >> +
> >> + rsvd_mask = vtd_spte_rsvd[level];
> >
> >
> > Looking at the code it is not clear to me why this assertion is
> > valid. It looks like we are picking up fields from guest-set
> > configuration (probably in-memory data structures). So we can't
> > assert() here -- we need to do whatever the real hardware does
> > if these fields are set to an incorrect value, or at least something
> > sensible that doesn't crash QEMU.
>
> But touching vtd_spte_rsvd with level>=5 is even worse than
> assertion, I think. That's overflows the array.
Correct. We shouldn't do that. But we also should not just
assert().
> I don't know what the real hardware should do in this case.
Then we should find out... Hopefully the specs will say.
If they don't then we can do whatever is a reasonable
behaviour (eg treat like some other valid value).
thanks
-- PMM
