2016-11-15 11:02-0600, Tom Lendacky: > On 11/15/2016 8:39 AM, Radim Krčmář wrote: >> 2016-11-09 18:37-0600, Tom Lendacky: >>> Since DMA addresses will effectively look like 48-bit addresses when the >>> memory encryption mask is set, SWIOTLB is needed if the DMA mask of the >>> device performing the DMA does not support 48-bits. SWIOTLB will be >>> initialized to create un-encrypted bounce buffers for use by these devices. >>> >>> Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx> >>> --- >>> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c >>> @@ -64,13 +66,15 @@ static struct dma_map_ops swiotlb_dma_ops = { >>> * pci_swiotlb_detect_override - set swiotlb to 1 if necessary >>> * >>> * This returns non-zero if we are forced to use swiotlb (by the boot >>> - * option). >>> + * option). If memory encryption is enabled then swiotlb will be set >>> + * to 1 so that bounce buffers are allocated and used for devices that >>> + * do not support the addressing range required for the encryption mask. >>> */ >>> int __init pci_swiotlb_detect_override(void) >>> { >>> int use_swiotlb = swiotlb | swiotlb_force; >>> >>> - if (swiotlb_force) >>> + if (swiotlb_force || sme_me_mask) >>> swiotlb = 1; >>> >>> return use_swiotlb; >> >> We want to return 1 even if only sme_me_mask is 1, because the return >> value is used for detection. The following would be less obscure, IMO: >> >> if (swiotlb_force || sme_me_mask) >> swiotlb = 1; >> >> return swiotlb; > > If we do that then all DMA would go through the swiotlb bounce buffers. No, that is decided for example in swiotlb_map_page() and we need to call pci_swiotlb_init() to register that function. > By setting swiotlb to 1 we indicate that the bounce buffers will be > needed for those devices that can't support the addressing range when > the encryption bit is set (48 bit DMA). But if the device can support > the addressing range we won't use the bounce buffers. If we return 0 here, then pci_swiotlb_init() will not be called => dma_ops won't be set to swiotlb_dma_ops => we won't use bounce buffers. >> We setup encrypted swiotlb and then decrypt it, but sometimes set it up >> decrypted (late_alloc) ... why isn't the swiotlb set up decrypted >> directly? > > When swiotlb is allocated in swiotlb_init(), it is too early to make > use of the api to the change the page attributes. Because of this, > the callback to make those changes is needed. Thanks. (I don't know page table setup enough to see a lesser evil. :]) >>> @@ -541,7 +583,7 @@ static phys_addr_t >>> map_single(struct device *hwdev, phys_addr_t phys, size_t size, >>> enum dma_data_direction dir) >>> { >>> - dma_addr_t start_dma_addr = phys_to_dma(hwdev, io_tlb_start); >>> + dma_addr_t start_dma_addr = swiotlb_phys_to_dma(hwdev, io_tlb_start); >> >> We have decrypted io_tlb_start before, so shouldn't its physical address >> be saved without the sme bit? (Which changes a lot ...) > > I'm not sure what you mean here, can you elaborate a bit more? The C-bit (sme bit) is a part of the physical address. If we know that a certain physical page should be accessed as unencrypted (the bounce buffer) then the C-bit is 0. I'm wondering why we save the physical address with the C-bit set when we know that it can't be accessed that way (because we remove it every time). The naming is a bit confusing, because physical addresses are actually virtualized by SME -- maybe we should be calling them SME addresses? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>