diff mbox

[v18,2/2] drm/i915/selftests: Introduce live tests of private PAT management

Message ID 1506014935-2012-2-git-send-email-zhi.a.wang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wang, Zhi A Sept. 21, 2017, 5:28 p.m. UTC
Introduce two live tests of private PAT management:

igt_ppat_init - This test is to check if all the PPAT configurations are
written into HW.

igt_ppat_get - This test performs several sub-tests on intel_ppat_get()
and intel_ppat_put().

The "perfect match" test case will try to get a PPAT entry with an existing
value, then check if the returned PPAT entry is the same one.

The "alloc entries" test case will run out of PPAT table, and check if all
the requested values are put into the newly allocated PPAT entries.

The negative test case will try to generate a new PPAT value, and get it
when PPAT table is full.

The "partial match" test case will generate a parital matched value from
the existing PPAT table and try to match it.

The "re-alloc" test case will try to free and then allocate a new entry
when the PPAT table is full.

The "put entries" test case will free all the PPAT entries that allocated
in "alloc entries" test case. It will check if the values of freed PPAT
entries turn into ppat->clear_value.

v18:

- Refine the test to catch a corner case.

v10:

- Refine code structure.
- Introduce "re-alloc" test case. (Chris)

v9:

- Refine generate_new_value(). (Chris)
- Refine failure output. (Chris)
- Refine test flow of "perfect match". (Chris)
- Introduce another negative test case after "partial match". (Chris)

v8:

- Remove noisy output. (Chris)
- Add negative test case. (Chris)

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 372 ++++++++++++++++++++++++++
 1 file changed, 372 insertions(+)

Comments

Joonas Lahtinen Sept. 22, 2017, 10:34 a.m. UTC | #1
On Fri, 2017-09-22 at 01:28 +0800, Zhi Wang wrote:
> Introduce two live tests of private PAT management:
> 
> igt_ppat_init - This test is to check if all the PPAT configurations are
> written into HW.
> 
> igt_ppat_get - This test performs several sub-tests on intel_ppat_get()
> and intel_ppat_put().
> 
> The "perfect match" test case will try to get a PPAT entry with an existing
> value, then check if the returned PPAT entry is the same one.
> 
> The "alloc entries" test case will run out of PPAT table, and check if all
> the requested values are put into the newly allocated PPAT entries.
> 
> The negative test case will try to generate a new PPAT value, and get it
> when PPAT table is full.
> 
> The "partial match" test case will generate a parital matched value from
> the existing PPAT table and try to match it.
> 
> The "re-alloc" test case will try to free and then allocate a new entry
> when the PPAT table is full.
> 
> The "put entries" test case will free all the PPAT entries that allocated
> in "alloc entries" test case. It will check if the values of freed PPAT
> entries turn into ppat->clear_value.
> 
> v18:
> 
> - Refine the test to catch a corner case.
> 
> v10:
> 
> - Refine code structure.
> - Introduce "re-alloc" test case. (Chris)
> 
> v9:
> 
> - Refine generate_new_value(). (Chris)
> - Refine failure output. (Chris)
> - Refine test flow of "perfect match". (Chris)
> - Introduce another negative test case after "partial match". (Chris)
> 
> v8:
> 
> - Remove noisy output. (Chris)
> - Add negative test case. (Chris)
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

<SNIP>

> +static int igt_ppat_check(void *arg)
> +{
> +	struct drm_i915_private *i915 = arg;
> +	int ret;
> +
> +	if (!i915->ppat.max_entries)
> +		return 0;
> +
> +	if (INTEL_GEN(i915) >= 10)
> +		ret = check_cnl_ppat(i915);
> +	else
> +		ret = check_bdw_ppat(i915);
> +
> +	if (ret)
> +		pr_err("check PPAT failed\n");

This is just extra noise, both individual dest already say check PPAT
failed.

> +
> +	return ret;
> +}
> +
> +static bool value_is_new(struct intel_ppat *ppat, u8 value)

Bit overly generic name for selftests/i915_gem_gtt.c, how about
'ppat_is_new' or 'ppat_find_match_all'.

> +{
> +	int i;
> +
> +	for_each_set_bit(i, ppat->used, ppat->max_entries) {
> +		if (value != ppat->entries[i].value)

		if (value == ppat->entries[i])
			return i;

And drop the braces around for.

	return -ENOENT;

> +static bool value_for_partial_test(struct intel_ppat *ppat, u8 value)

Maybe 'ppat_find_match_ca', similar changes to above function to return
index.

> +{
> +	int i;
> +
> +	if (!value_is_new(ppat, value))
> +		return false;
> +
> +	/*
> +	 * At least, there should be one entry whose cache attribute is
> +	 * same with the required value.
> +	 */

Comment says what the code does, so can be dropped.

> +	for_each_set_bit(i, ppat->used, ppat->max_entries) {
> +		if (GEN8_PPAT_GET_CA(value) !=
> +		    GEN8_PPAT_GET_CA(ppat->entries[i].value))

Again '==' and return true.

> +			continue;
> +
> +		return true;
> +	}
> +	return false;
> +}
> +
> +static bool value_for_negative_test(struct intel_ppat *ppat, u8 value)

Maybe 'ppat_find_not_match_ca', same with returning index / -ENOENT.

> +{
> +	int i;
> +
> +	if (!value_is_new(ppat, value))
> +		return false;
> +
> +	/*
> +	 * cache attribute has to be different, so i915_ppat_get() would
> +	 * allocate a new entry.
> +	 */

Ditto.

> +	for_each_set_bit(i, ppat->used, ppat->max_entries) {
> +		if (GEN8_PPAT_GET_CA(value) ==
> +		    GEN8_PPAT_GET_CA(ppat->entries[i].value))
> +			return false;

You can drop the braces, here too.

> +	}
> +	return true;
> +}
> +
> +static u8 generate_new_value(struct intel_ppat *ppat,
> +			     bool (*check_value)(struct intel_ppat *, u8))

'ppat_generate_new' or so. Also, by definition of _new, this function
should be doing the check if it already exists, not the filter function
(unnecessary code duplication).

> +{
> +	u8 ca[] = { GEN8_PPAT_WB, GEN8_PPAT_WT, GEN8_PPAT_UC, GEN8_PPAT_WC };
> +	u8 tc[] = { GEN8_PPAT_LLC, GEN8_PPAT_LLCELLC, GEN8_PPAT_LLCeLLC };
> +	u8 age[] = { GEN8_PPAT_AGE(3), GEN8_PPAT_AGE(2), GEN8_PPAT_AGE(1),
> +		     GEN8_PPAT_AGE(0) };
> +	int ca_index, tc_index, age_index;
> +	u8 value;
> +
> +#define for_each_ppat_attr(ca_index, tc_index, age_index) \
> +	for ((ca_index) = 0 ; (ca_index) < ARRAY_SIZE(ca); (ca_index)++) \
> +	for ((tc_index) = 0; (tc_index) < ARRAY_SIZE(tc); (tc_index)++) \
> +	for ((age_index) = 0; (age_index) < ARRAY_SIZE(age); (age_index)++)
> +
> +	for_each_ppat_attr(ca_index, tc_index, age_index) {
> +		value = age[age_index] | ca[ca_index] | tc[tc_index];
> +		if (check_value(ppat, value))
> +			return value;
> +	}
> +#undef for_each_ppat_attr
> +	return 0;

	-ENOSPC?

> +}
> +
> +static const struct intel_ppat_entry *
> +generate_and_check_new_value(struct intel_ppat *ppat)
> +{
> +	struct drm_i915_private *i915 = ppat->i915;
> +	const struct intel_ppat_entry *entry;
> +	u8 value;

Maybe swap these two declarations for extra fine tune.

> +	DECLARE_BITMAP(used, INTEL_MAX_PPAT_ENTRIES);
> +
> +	value = generate_new_value(ppat, value_is_new);
> +	if (!value) {
> +		pr_err("fail to generate new value\n");
> +		return ERR_PTR(-EINVAL);
> +	}

It's better to propagate errors, as much as possible, will be easier
for the next person to extend the tests, possibly adding new error
conditions.

	value = ppat_generate_new(..);
	if (IS_ERR(value)) {
		..
		return ERR_PTR(value);
	}


> +	bitmap_copy(used, ppat->used, ppat->max_entries);
> +
> +	entry = intel_ppat_get(i915, value);
> +	if (IS_ERR(entry)) {
> +		pr_err("fail to get new entry\n");
> +		return ERR_PTR(-EINVAL);
> +	}

Shouldn't the selftest very strictly differentiate between, "unable to
run test" (resources are in use, for example) and "i915 driver failed
the test". Here both problems translate into -EINVAL. Please check the
drm_mm tests for examples.

> +
> +	if (entry->value != value) {
> +		pr_err("value is not expected, expected %x found %x\n",
> +				value, entry->value);
> +		goto err;
> +	}
> +
> +	if (bitmap_equal(used, ppat->used, ppat->max_entries)) {
> +		pr_err("fail to alloc a new entry\n");
> +		goto err;
> +	}

This would be the more logical test to do first, I don't think it gets
triggered otherwise.

> +
> +	return entry;

Newline here.

> +err:
> +	intel_ppat_put(entry);

Usually here too.

> +	return ERR_PTR(-EINVAL);
> +}
> +
> +static int put_and_check_entry(const struct intel_ppat_entry *entry)

add word 'ppat' somewhere.

> +{
> +	intel_ppat_put(entry);
> +
> +	if (entry->value != entry->ppat->clear_value) {
> +		pr_err("fail to put ppat value\n");

Wouldn't 'ppat value was not cleared' be more informative, if
intel_ppat_put returned error, this would be appropriate.

> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int perform_perfect_match_test(struct intel_ppat *ppat)

s/perform/ppat/

> +{
> +	struct drm_i915_private *i915 = ppat->i915;
> +	const struct intel_ppat_entry *entry;
> +	int i;
> +
> +	for_each_set_bit(i, ppat->used, ppat->max_entries) {
> +		entry = intel_ppat_get(i915, ppat->entries[i].value);
> +		if (IS_ERR(entry))
> +			return PTR_ERR(entry);
> +

It's not obvious without looking elsewhere in the code, so it might be
worth mentioning that all entries have been generated to be unique.

		/* All generated PPAT entries are unique */

> +		if (entry != &ppat->entries[i]) {
> +			pr_err("entry is not expected\n");

Do add "goto err" and this becomes.

		if (entry != ..)
			goto err;

Just by glancing at the code, you can see we're doing an extremely 
simple test. Then, without disrupting the code flow, at err: label, you
can add details about what was expected and what we got.


> +			intel_ppat_put(entry);
> +			return -EINVAL;
> +		}
> +		intel_ppat_put(entry);
> +	}
> +	return 0;
> +}
> +
> +static int perform_negative_test(struct intel_ppat *ppat)
> +{
> +	struct drm_i915_private *i915 = ppat->i915;
> +	const struct intel_ppat_entry *entry;
> +	u8 value;
> +
> +	value = generate_new_value(ppat, value_for_negative_test);
> +	if (!value) {
> +		pr_err("fail to generate new value for negative test 2\n");
		
		Drop 'test 2'.

> +		return -EINVAL;
> +	}
> +
> +	entry = intel_ppat_get(i915, value);
> +	if (!IS_ERR(entry) || PTR_ERR(entry) != -ENOSPC) {
> +		pr_err("fail on negative test\n");
> +		return -EINVAL;
> +	}

Again, differentiate "could not run logic" vs. "failure in logic". Lets
try to propagate the error to the topmost function, and we have
possibility to report SKIP vs FAILURE to I-G-T.

> +	return 0;
> +}
> +
> +static int perform_partial_match_test(struct intel_ppat *ppat)
> +{
> +	struct drm_i915_private *i915 = ppat->i915;
> +	const struct intel_ppat_entry *entry;
> +	u8 value;
> +
> +	value = generate_new_value(ppat, value_for_partial_test);

These will read much better when the filter function has meaningful
name.

> +	if (!value) {
> +		pr_err("fail to generate new value for partial test\n");
> +		return -EINVAL;

This would be a 'could not perform test' instead of test failure.

> +	}
> +
> +	entry = intel_ppat_get(i915, value);
> +	if (IS_ERR(entry)) {
> +		pr_err("fail to get new entry\n");
> +		return PTR_ERR(entry);

This would be too a 'don't have space to execute test error', so here
propagating the error would be good idea.

> +	}
> +
> +	if (!(entry->value != value &&
> +	    GEN8_PPAT_GET_CA(entry->value) == GEN8_PPAT_GET_CA(value))) {

Wrong indent, you're negating both, so need to indent to the inner '('

Instead, negating the whole thing allows dropping the fancy indent and
makes the condition much more understandable:
	
	if (entry->value == value || .._CA != _CA

'We should not get equal value, and we should not get different _CA'

That's what partial match is about :)


> +		pr_err("value is not expected, expected %x found %x\n",
> +				value, entry->value);
> +		intel_ppat_put(entry);
> +		return -EINVAL;
> +	}
> +
> +	intel_ppat_put(entry);
> +	return 0;
> +}
> +
> +static int igt_ppat_get(void *arg)
> +{
> +	struct drm_i915_private *i915 = arg;
> +	struct intel_ppat *ppat = &i915->ppat;
> +	const struct intel_ppat_entry **entries, **p;
> +	const struct intel_ppat_entry *entry;
> +	unsigned int size = 0;
> +	int i, ret;
> +
> +	if (!ppat->max_entries)
> +		return 0;
> +
> +	ret = igt_ppat_check(i915);
> +	if (ret)
> +		return ret;
> +
> +	/* case 1: alloc new entries */
> +	entries = NULL;
> +	ret = 0;

This ret = 0 is not needed, we already have preceding
'if (ret) return ret;'

Then drop the extra newline, we're looping to change entries, it should
be close to the beginning of loop.

> +
> +	while (!bitmap_full(ppat->used, ppat->max_entries)) {
> +		p = krealloc(entries, (size + 1) *
> +				   sizeof(struct intel_ppat_entry *),
> +				   GFP_KERNEL);

Weird indent, you can even reduce line length with
(size + 1)*sizeof(*entries). 

> +		if (!p) {
> +			ret = -ENOMEM;
> +			goto ppat_put;
> +		}
> +
> +		entries = p;
> +
> +		p = &entries[size++];
> +		*p = NULL;

I think we could allocate at one go, it's literally a couple of
pointers (ppat->max_entries), I think this is bit much for allocating
pointers.

> +
> +		entry = generate_and_check_new_value(ppat);
> +		if (IS_ERR(entry)) {
> +			ret = PTR_ERR(entry);
> +			pr_err("fail on alloc new entries test\n");
> +			goto ppat_put;
> +		}
> +		*p = entry;
> +	}
> +
> +	/* case 2: perfect match */
> +	ret = perform_perfect_match_test(ppat);
> +	if (ret) {
> +		pr_err("fail on perfect match test\n");
> +		return ret;
> +	}
> +
> +	/* case 3: negative test 1, suppose PPAT table is full now */
> +	ret = perform_negative_test(ppat);
> +	if (ret) {
> +		pr_err("fail on negative test 1\n");
> +		goto ppat_put;
> +	}
> +
> +	/* case 4: partial match */
> +	ret = perform_partial_match_test(ppat);
> +	if (ret) {
> +		pr_err("fail on partial match test\n");
> +		goto ppat_put;
> +	}
> +
> +	/* case 3: negative test 2, suppose PPAT table is still full now */
> +	ret = perform_negative_test(ppat);
> +	if (ret) {
> +		pr_err("fail on negative test 2\n");
> +		goto ppat_put;
> +	}
> +
> +	/* case 5: re-alloc test, make a hole and it should work again */
> +	if (entries) {

entries should not be NULL here, we're skipping function krealloc
fails. So causes extra indent.

> +		for (i = 0; i < size; i++) {
> +			entry = entries[i];
> +
> +			ret = put_and_check_entry(entry);
> +			entries[i] = NULL;
> +			if (ret) {
> +				pr_err("fail on re-alloc test\n");
> +				goto ppat_put;
> +			}
> +
> +			entry = generate_and_check_new_value(ppat);
> +			if (IS_ERR(entry)) {
> +				ret = PTR_ERR(entry);
> +				pr_err("fail on re-alloc test\n");
> +				goto ppat_put;
> +			}
> +			entries[i] = entry;
> +		}
> +	}
> +
> +ppat_put:
> +	if (entries) {

This extra indent can go when we we have goto out; at the allocation
error for entries.

> +		for (i = 0; i < size; i++) {
> +			if (IS_ERR(entries[i]) || !entries[i])

IS_ERR_OR_NULL

> +				continue;
> +
> +			if (ret)
> +				intel_ppat_put(entry);
> +			else
> +				ret = put_and_check_entry(entries[i]);
> +		}
> +		kfree(entries);
> +		entries = NULL;
> +	}

out:

> +	if (ret)
> +		return ret;
> +
> +	return igt_ppat_check(i915);
> +}
> +

With the modifications we should be very much ready with the kselftest,
as I see it.

Regards, Joonas
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 6b132ca..75cb2d6 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1094,6 +1094,376 @@  static int igt_ggtt_page(void *arg)
 	return err;
 }
 
+static int check_cnl_ppat(struct drm_i915_private *dev_priv)
+{
+	struct intel_ppat *ppat = &dev_priv->ppat;
+	int i;
+
+	for (i = 0; i < ppat->max_entries; i++) {
+		u32 value = I915_READ(GEN10_PAT_INDEX(i));
+
+		if (value != ppat->entries[i].value) {
+			pr_err("check PPAT failed: expected %x found %x\n",
+					ppat->entries[i].value, value);
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+static int check_bdw_ppat(struct drm_i915_private *dev_priv)
+{
+	struct intel_ppat *ppat = &dev_priv->ppat;
+	u64 pat, hw_pat;
+	int i;
+
+	pat = hw_pat = 0;
+
+	for (i = 0; i < ppat->max_entries; i++)
+		pat |= GEN8_PPAT(i, ppat->entries[i].value);
+
+	hw_pat = I915_READ(GEN8_PRIVATE_PAT_HI);
+	hw_pat <<= 32;
+	hw_pat |= I915_READ(GEN8_PRIVATE_PAT_LO);
+
+	if (pat != hw_pat) {
+		pr_err("check PPAT failed: expected %llx found %llx\n",
+				pat, hw_pat);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int igt_ppat_check(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	int ret;
+
+	if (!i915->ppat.max_entries)
+		return 0;
+
+	if (INTEL_GEN(i915) >= 10)
+		ret = check_cnl_ppat(i915);
+	else
+		ret = check_bdw_ppat(i915);
+
+	if (ret)
+		pr_err("check PPAT failed\n");
+
+	return ret;
+}
+
+static bool value_is_new(struct intel_ppat *ppat, u8 value)
+{
+	int i;
+
+	for_each_set_bit(i, ppat->used, ppat->max_entries) {
+		if (value != ppat->entries[i].value)
+			continue;
+
+		return false;
+	}
+	return true;
+}
+
+static bool value_for_partial_test(struct intel_ppat *ppat, u8 value)
+{
+	int i;
+
+	if (!value_is_new(ppat, value))
+		return false;
+
+	/*
+	 * At least, there should be one entry whose cache attribute is
+	 * same with the required value.
+	 */
+	for_each_set_bit(i, ppat->used, ppat->max_entries) {
+		if (GEN8_PPAT_GET_CA(value) !=
+		    GEN8_PPAT_GET_CA(ppat->entries[i].value))
+			continue;
+
+		return true;
+	}
+	return false;
+}
+
+static bool value_for_negative_test(struct intel_ppat *ppat, u8 value)
+{
+	int i;
+
+	if (!value_is_new(ppat, value))
+		return false;
+
+	/*
+	 * cache attribute has to be different, so i915_ppat_get() would
+	 * allocate a new entry.
+	 */
+	for_each_set_bit(i, ppat->used, ppat->max_entries) {
+		if (GEN8_PPAT_GET_CA(value) ==
+		    GEN8_PPAT_GET_CA(ppat->entries[i].value))
+			return false;
+	}
+	return true;
+}
+
+static u8 generate_new_value(struct intel_ppat *ppat,
+			     bool (*check_value)(struct intel_ppat *, u8))
+{
+	u8 ca[] = { GEN8_PPAT_WB, GEN8_PPAT_WT, GEN8_PPAT_UC, GEN8_PPAT_WC };
+	u8 tc[] = { GEN8_PPAT_LLC, GEN8_PPAT_LLCELLC, GEN8_PPAT_LLCeLLC };
+	u8 age[] = { GEN8_PPAT_AGE(3), GEN8_PPAT_AGE(2), GEN8_PPAT_AGE(1),
+		     GEN8_PPAT_AGE(0) };
+	int ca_index, tc_index, age_index;
+	u8 value;
+
+#define for_each_ppat_attr(ca_index, tc_index, age_index) \
+	for ((ca_index) = 0 ; (ca_index) < ARRAY_SIZE(ca); (ca_index)++) \
+	for ((tc_index) = 0; (tc_index) < ARRAY_SIZE(tc); (tc_index)++) \
+	for ((age_index) = 0; (age_index) < ARRAY_SIZE(age); (age_index)++)
+
+	for_each_ppat_attr(ca_index, tc_index, age_index) {
+		value = age[age_index] | ca[ca_index] | tc[tc_index];
+		if (check_value(ppat, value))
+			return value;
+	}
+#undef for_each_ppat_attr
+	return 0;
+}
+
+static const struct intel_ppat_entry *
+generate_and_check_new_value(struct intel_ppat *ppat)
+{
+	struct drm_i915_private *i915 = ppat->i915;
+	const struct intel_ppat_entry *entry;
+	u8 value;
+	DECLARE_BITMAP(used, INTEL_MAX_PPAT_ENTRIES);
+
+	value = generate_new_value(ppat, value_is_new);
+	if (!value) {
+		pr_err("fail to generate new value\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	bitmap_copy(used, ppat->used, ppat->max_entries);
+
+	entry = intel_ppat_get(i915, value);
+	if (IS_ERR(entry)) {
+		pr_err("fail to get new entry\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (entry->value != value) {
+		pr_err("value is not expected, expected %x found %x\n",
+				value, entry->value);
+		goto err;
+	}
+
+	if (bitmap_equal(used, ppat->used, ppat->max_entries)) {
+		pr_err("fail to alloc a new entry\n");
+		goto err;
+	}
+
+	return entry;
+err:
+	intel_ppat_put(entry);
+	return ERR_PTR(-EINVAL);
+}
+
+static int put_and_check_entry(const struct intel_ppat_entry *entry)
+{
+	intel_ppat_put(entry);
+
+	if (entry->value != entry->ppat->clear_value) {
+		pr_err("fail to put ppat value\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int perform_perfect_match_test(struct intel_ppat *ppat)
+{
+	struct drm_i915_private *i915 = ppat->i915;
+	const struct intel_ppat_entry *entry;
+	int i;
+
+	for_each_set_bit(i, ppat->used, ppat->max_entries) {
+		entry = intel_ppat_get(i915, ppat->entries[i].value);
+		if (IS_ERR(entry))
+			return PTR_ERR(entry);
+
+		if (entry != &ppat->entries[i]) {
+			pr_err("entry is not expected\n");
+			intel_ppat_put(entry);
+			return -EINVAL;
+		}
+		intel_ppat_put(entry);
+	}
+	return 0;
+}
+
+static int perform_negative_test(struct intel_ppat *ppat)
+{
+	struct drm_i915_private *i915 = ppat->i915;
+	const struct intel_ppat_entry *entry;
+	u8 value;
+
+	value = generate_new_value(ppat, value_for_negative_test);
+	if (!value) {
+		pr_err("fail to generate new value for negative test 2\n");
+		return -EINVAL;
+	}
+
+	entry = intel_ppat_get(i915, value);
+	if (!IS_ERR(entry) || PTR_ERR(entry) != -ENOSPC) {
+		pr_err("fail on negative test\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int perform_partial_match_test(struct intel_ppat *ppat)
+{
+	struct drm_i915_private *i915 = ppat->i915;
+	const struct intel_ppat_entry *entry;
+	u8 value;
+
+	value = generate_new_value(ppat, value_for_partial_test);
+	if (!value) {
+		pr_err("fail to generate new value for partial test\n");
+		return -EINVAL;
+	}
+
+	entry = intel_ppat_get(i915, value);
+	if (IS_ERR(entry)) {
+		pr_err("fail to get new entry\n");
+		return PTR_ERR(entry);
+	}
+
+	if (!(entry->value != value &&
+	    GEN8_PPAT_GET_CA(entry->value) == GEN8_PPAT_GET_CA(value))) {
+		pr_err("value is not expected, expected %x found %x\n",
+				value, entry->value);
+		intel_ppat_put(entry);
+		return -EINVAL;
+	}
+
+	intel_ppat_put(entry);
+	return 0;
+}
+
+static int igt_ppat_get(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_ppat *ppat = &i915->ppat;
+	const struct intel_ppat_entry **entries, **p;
+	const struct intel_ppat_entry *entry;
+	unsigned int size = 0;
+	int i, ret;
+
+	if (!ppat->max_entries)
+		return 0;
+
+	ret = igt_ppat_check(i915);
+	if (ret)
+		return ret;
+
+	/* case 1: alloc new entries */
+	entries = NULL;
+	ret = 0;
+
+	while (!bitmap_full(ppat->used, ppat->max_entries)) {
+		p = krealloc(entries, (size + 1) *
+				   sizeof(struct intel_ppat_entry *),
+				   GFP_KERNEL);
+		if (!p) {
+			ret = -ENOMEM;
+			goto ppat_put;
+		}
+
+		entries = p;
+
+		p = &entries[size++];
+		*p = NULL;
+
+		entry = generate_and_check_new_value(ppat);
+		if (IS_ERR(entry)) {
+			ret = PTR_ERR(entry);
+			pr_err("fail on alloc new entries test\n");
+			goto ppat_put;
+		}
+		*p = entry;
+	}
+
+	/* case 2: perfect match */
+	ret = perform_perfect_match_test(ppat);
+	if (ret) {
+		pr_err("fail on perfect match test\n");
+		return ret;
+	}
+
+	/* case 3: negative test 1, suppose PPAT table is full now */
+	ret = perform_negative_test(ppat);
+	if (ret) {
+		pr_err("fail on negative test 1\n");
+		goto ppat_put;
+	}
+
+	/* case 4: partial match */
+	ret = perform_partial_match_test(ppat);
+	if (ret) {
+		pr_err("fail on partial match test\n");
+		goto ppat_put;
+	}
+
+	/* case 3: negative test 2, suppose PPAT table is still full now */
+	ret = perform_negative_test(ppat);
+	if (ret) {
+		pr_err("fail on negative test 2\n");
+		goto ppat_put;
+	}
+
+	/* case 5: re-alloc test, make a hole and it should work again */
+	if (entries) {
+		for (i = 0; i < size; i++) {
+			entry = entries[i];
+
+			ret = put_and_check_entry(entry);
+			entries[i] = NULL;
+			if (ret) {
+				pr_err("fail on re-alloc test\n");
+				goto ppat_put;
+			}
+
+			entry = generate_and_check_new_value(ppat);
+			if (IS_ERR(entry)) {
+				ret = PTR_ERR(entry);
+				pr_err("fail on re-alloc test\n");
+				goto ppat_put;
+			}
+			entries[i] = entry;
+		}
+	}
+
+ppat_put:
+	if (entries) {
+		for (i = 0; i < size; i++) {
+			if (IS_ERR(entries[i]) || !entries[i])
+				continue;
+
+			if (ret)
+				intel_ppat_put(entry);
+			else
+				ret = put_and_check_entry(entries[i]);
+		}
+		kfree(entries);
+		entries = NULL;
+	}
+	if (ret)
+		return ret;
+
+	return igt_ppat_check(i915);
+}
+
 static void track_vma_bind(struct i915_vma *vma)
 {
 	struct drm_i915_gem_object *obj = vma->obj;
@@ -1560,6 +1930,8 @@  int i915_gem_gtt_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_ggtt_pot),
 		SUBTEST(igt_ggtt_fill),
 		SUBTEST(igt_ggtt_page),
+		SUBTEST(igt_ppat_check),
+		SUBTEST(igt_ppat_get),
 	};
 
 	GEM_BUG_ON(offset_in_page(i915->ggtt.base.total));